daandemeyer / rpms / systemd

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