From 1a05ff4948d778280ec155a9abe69d3360bfddd9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 17 Oct 2018 18:36:24 +0200 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. CVE-2018-15686 LP: #1796402 https://bugzilla.redhat.com/show_bug.cgi?id=1639071 (cherry picked from commit 8948b3415d762245ebf5e19d80b97d4d8cc208c1) CVE: CVE-2018-15686 Upstream-Status: Backport Signed-off-by: Chen Qi --- src/core/job.c | 19 +++++++++++-------- src/core/manager.c | 44 ++++++++++++++++++++------------------------ src/core/unit.c | 34 ++++++++++++++++++---------------- src/core/unit.h | 2 +- 4 files changed, 50 insertions(+), 49 deletions(-) diff --git a/src/core/job.c b/src/core/job.c index 734756b..8552ffb 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -10,6 +10,7 @@ #include "dbus-job.h" #include "dbus.h" #include "escape.h" +#include "fileio.h" #include "job.h" #include "log.h" #include "macro.h" @@ -1091,24 +1092,26 @@ int job_serialize(Job *j, FILE *f) { } int job_deserialize(Job *j, FILE *f) { + int r; + assert(j); assert(f); 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 3a7f0c4..a5780c9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -3171,22 +3171,19 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { m->n_reloading++; for (;;) { - char line[LINE_MAX]; + _cleanup_free_ char *line = NULL; const char *val, *l; - if (!fgets(line, sizeof(line), f)) { - if (feof(f)) - r = 0; - else - r = -errno; - + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) { + log_error_errno(r, "Failed to read serialization line: %m"); goto finish; } + if (r == 0) + break; - char_array_0(line); l = strstrip(line); - - if (l[0] == 0) + if (isempty(l)) /* end marker */ break; if ((val = startswith(l, "current-job-id="))) { @@ -3353,29 +3350,31 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { } for (;;) { - Unit *u; - char name[UNIT_NAME_MAX+2]; + _cleanup_free_ char *line = NULL; const char* unit_name; + Unit *u; /* Start marker */ - if (!fgets(name, sizeof(name), f)) { - if (feof(f)) - r = 0; - else - r = -errno; - + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) { + log_error_errno(r, "Failed to read serialization line: %m"); goto finish; } + if (r == 0) + break; - char_array_0(name); - unit_name = strstrip(name); + unit_name = strstrip(line); r = manager_load_unit(m, unit_name, NULL, NULL, &u); if (r < 0) { log_notice_errno(r, "Failed to load unit \"%s\", skipping deserialization: %m", unit_name); if (r == -ENOMEM) goto finish; - unit_deserialize_skip(f); + + r = unit_deserialize_skip(f); + if (r < 0) + goto finish; + continue; } @@ -3388,9 +3387,6 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { } finish: - if (ferror(f)) - r = -EIO; - assert(m->n_reloading > 0); m->n_reloading--; diff --git a/src/core/unit.c b/src/core/unit.c index 7da963a..e98c9c4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3380,21 +3380,19 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { assert(fds); for (;;) { - char line[LINE_MAX], *l, *v; + _cleanup_free_ char *line = NULL; CGroupIPAccountingMetric m; + 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 */ + break; - char_array_0(line); l = strstrip(line); - - /* End marker */ - if (isempty(l)) + if (isempty(l)) /* End marker */ break; k = strcspn(l, "="); @@ -3671,23 +3669,27 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { return 0; } -void unit_deserialize_skip(FILE *f) { +int unit_deserialize_skip(FILE *f) { + int r; assert(f); /* Skip serialized data for this unit. We don't know what it is. */ for (;;) { - char line[LINE_MAX], *l; + _cleanup_free_ char *line = NULL; + char *l; - if (!fgets(line, sizeof line, f)) - return; + 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 (isempty(l)) - return; + return 1; } } diff --git a/src/core/unit.h b/src/core/unit.h index 06321bb..51c7aaa 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -684,7 +684,7 @@ bool unit_can_serialize(Unit *u) _pure_; int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs); int unit_deserialize(Unit *u, FILE *f, FDSet *fds); -void unit_deserialize_skip(FILE *f); +int unit_deserialize_skip(FILE *f); int unit_serialize_item(Unit *u, FILE *f, const char *key, const char *value); int unit_serialize_item_escaped(Unit *u, FILE *f, const char *key, const char *value); -- 2.7.4