You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
182 lines
5.8 KiB
182 lines
5.8 KiB
5 years ago
|
From 650d5c39753d095f8ae6cff418306cf1a6b8fd25 Mon Sep 17 00:00:00 2001
|
||
|
From: Jan Synacek <jsynacek@redhat.com>
|
||
|
Date: Fri, 2 Nov 2018 08:40:40 +0100
|
||
|
Subject: [PATCH] =?UTF-8?q?core:=20when=20deserializing=20state=20always?=
|
||
|
=?UTF-8?q?=20use=20read=5Fline(=E2=80=A6,=20LONG=5FLINE=5FMAX,=20?=
|
||
|
=?UTF-8?q?=E2=80=A6)?=
|
||
|
MIME-Version: 1.0
|
||
|
Content-Type: text/plain; charset=UTF-8
|
||
|
Content-Transfer-Encoding: 8bit
|
||
|
|
||
|
This should be much better than fgets(), as we can read substantially
|
||
|
longer lines and overly long lines result in proper errors.
|
||
|
|
||
|
Fixes a vulnerability discovered by Jann Horn at Google.
|
||
|
|
||
|
(cherry picked from commit 8948b3415d762245ebf5e19d80b97d4d8cc208c1)
|
||
|
|
||
|
Resolves: CVE-2018-15686
|
||
|
|
||
|
[jsynacek] There were more commits in the pull request of which the cherry
|
||
|
picked commit was a part of, see
|
||
|
https://github.com/systemd/systemd/pull/10519/commits.
|
||
|
I decided not to backport any of the remaining ones, because they were
|
||
|
mostly irrelevant to the actual fix.
|
||
|
---
|
||
|
src/core/job.c | 19 +++++++++++--------
|
||
|
src/core/manager.c | 38 ++++++++++++++++----------------------
|
||
|
src/core/unit.c | 17 ++++++++---------
|
||
|
3 files changed, 35 insertions(+), 39 deletions(-)
|
||
|
|
||
|
diff --git a/src/core/job.c b/src/core/job.c
|
||
|
index 275503169b..998b231ed9 100644
|
||
|
--- a/src/core/job.c
|
||
|
+++ b/src/core/job.c
|
||
|
@@ -38,6 +38,7 @@
|
||
|
#include "async.h"
|
||
|
#include "virt.h"
|
||
|
#include "dbus.h"
|
||
|
+#include "fileio.h"
|
||
|
|
||
|
Job* job_new_raw(Unit *unit) {
|
||
|
Job *j;
|
||
|
@@ -1035,23 +1036,25 @@ int job_serialize(Job *j, FILE *f, FDSet *fds) {
|
||
|
}
|
||
|
|
||
|
int job_deserialize(Job *j, FILE *f, FDSet *fds) {
|
||
|
+ int r = 0;
|
||
|
+
|
||
|
assert(j);
|
||
|
|
||
|
for (;;) {
|
||
|
- char line[LINE_MAX], *l, *v;
|
||
|
+ _cleanup_free_ char *line = NULL;
|
||
|
+ char *l, *v;
|
||
|
size_t k;
|
||
|
|
||
|
- if (!fgets(line, sizeof(line), f)) {
|
||
|
- if (feof(f))
|
||
|
- return 0;
|
||
|
- return -errno;
|
||
|
- }
|
||
|
+ r = read_line(f, LONG_LINE_MAX, &line);
|
||
|
+ if (r < 0)
|
||
|
+ return log_error_errno(r, "Failed to read serialization line: %m");
|
||
|
+ if (r == 0)
|
||
|
+ return 0;
|
||
|
|
||
|
- char_array_0(line);
|
||
|
l = strstrip(line);
|
||
|
|
||
|
/* End marker */
|
||
|
- if (l[0] == 0)
|
||
|
+ if (isempty(l))
|
||
|
return 0;
|
||
|
|
||
|
k = strcspn(l, "=");
|
||
|
diff --git a/src/core/manager.c b/src/core/manager.c
|
||
|
index 0466e4bb8a..73d6c81fdb 100644
|
||
|
--- a/src/core/manager.c
|
||
|
+++ b/src/core/manager.c
|
||
|
@@ -37,6 +37,7 @@
|
||
|
#include <sys/stat.h>
|
||
|
#include <dirent.h>
|
||
|
#include <sys/timerfd.h>
|
||
|
+#include <fileio.h>
|
||
|
|
||
|
#ifdef HAVE_AUDIT
|
||
|
#include <libaudit.h>
|
||
|
@@ -2559,21 +2560,19 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
|
||
|
m->n_reloading ++;
|
||
|
|
||
|
for (;;) {
|
||
|
- char line[LINE_MAX], *l;
|
||
|
+ _cleanup_free_ char *line = NULL;
|
||
|
+ char *l;
|
||
|
|
||
|
- if (!fgets(line, sizeof(line), f)) {
|
||
|
- if (feof(f))
|
||
|
- r = 0;
|
||
|
- else
|
||
|
- r = -errno;
|
||
|
|
||
|
- goto finish;
|
||
|
- }
|
||
|
+ r = read_line(f, LONG_LINE_MAX, &line);
|
||
|
+ if (r < 0)
|
||
|
+ return log_error_errno(r, "Failed to read serialization line: %m");
|
||
|
+ if (r == 0)
|
||
|
+ break;
|
||
|
|
||
|
- char_array_0(line);
|
||
|
l = strstrip(line);
|
||
|
|
||
|
- if (l[0] == 0)
|
||
|
+ if (isempty(l)) /* end marker */
|
||
|
break;
|
||
|
|
||
|
if (startswith(l, "current-job-id=")) {
|
||
|
@@ -2708,22 +2707,17 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
|
||
|
}
|
||
|
|
||
|
for (;;) {
|
||
|
+ _cleanup_free_ char *line = NULL;
|
||
|
Unit *u;
|
||
|
- char name[UNIT_NAME_MAX+2];
|
||
|
|
||
|
/* Start marker */
|
||
|
- if (!fgets(name, sizeof(name), f)) {
|
||
|
- if (feof(f))
|
||
|
- r = 0;
|
||
|
- else
|
||
|
- r = -errno;
|
||
|
-
|
||
|
- goto finish;
|
||
|
- }
|
||
|
-
|
||
|
- char_array_0(name);
|
||
|
+ r = read_line(f, LONG_LINE_MAX, &line);
|
||
|
+ if (r < 0)
|
||
|
+ return log_error_errno(r, "Failed to read serialization line: %m");
|
||
|
+ if (r == 0)
|
||
|
+ break;
|
||
|
|
||
|
- r = manager_load_unit(m, strstrip(name), NULL, NULL, &u);
|
||
|
+ r = manager_load_unit(m, strstrip(line), NULL, NULL, &u);
|
||
|
if (r < 0)
|
||
|
goto finish;
|
||
|
|
||
|
diff --git a/src/core/unit.c b/src/core/unit.c
|
||
|
index e8532a057d..37fac8db3a 100644
|
||
|
--- a/src/core/unit.c
|
||
|
+++ b/src/core/unit.c
|
||
|
@@ -2644,20 +2644,19 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) {
|
||
|
rt = (ExecRuntime**) ((uint8_t*) u + offset);
|
||
|
|
||
|
for (;;) {
|
||
|
- char line[LINE_MAX], *l, *v;
|
||
|
+ _cleanup_free_ char *line = NULL;
|
||
|
+ char *l, *v;
|
||
|
size_t k;
|
||
|
|
||
|
- if (!fgets(line, sizeof(line), f)) {
|
||
|
- if (feof(f))
|
||
|
- return 0;
|
||
|
- return -errno;
|
||
|
- }
|
||
|
+ r = read_line(f, LONG_LINE_MAX, &line);
|
||
|
+ if (r < 0)
|
||
|
+ return log_error_errno(r, "Failed to read serialization line: %m");
|
||
|
+ if (r == 0) /* eof */
|
||
|
+ return 0;
|
||
|
|
||
|
- char_array_0(line);
|
||
|
l = strstrip(line);
|
||
|
-
|
||
|
/* End marker */
|
||
|
- if (l[0] == 0)
|
||
|
+ if (isempty(l))
|
||
|
return 0;
|
||
|
|
||
|
k = strcspn(l, "=");
|