Brad Bishop | 1a4b7ee | 2018-12-16 17:11:34 -0800 | [diff] [blame] | 1 | From 1a05ff4948d778280ec155a9abe69d3360bfddd9 Mon Sep 17 00:00:00 2001 |
| 2 | From: Lennart Poettering <lennart@poettering.net> |
| 3 | Date: Wed, 17 Oct 2018 18:36:24 +0200 |
| 4 | Subject: [PATCH] =?UTF-8?q?core:=20when=20deserializing=20state=20always?= |
| 5 | =?UTF-8?q?=20use=20read=5Fline(=E2=80=A6,=20LONG=5FLINE=5FMAX,=20?= |
| 6 | =?UTF-8?q?=E2=80=A6)?= |
| 7 | MIME-Version: 1.0 |
| 8 | Content-Type: text/plain; charset=UTF-8 |
| 9 | Content-Transfer-Encoding: 8bit |
| 10 | |
| 11 | This should be much better than fgets(), as we can read substantially |
| 12 | longer lines and overly long lines result in proper errors. |
| 13 | |
| 14 | Fixes a vulnerability discovered by Jann Horn at Google. |
| 15 | |
| 16 | CVE-2018-15686 |
| 17 | LP: #1796402 |
| 18 | https://bugzilla.redhat.com/show_bug.cgi?id=1639071 |
| 19 | |
| 20 | (cherry picked from commit 8948b3415d762245ebf5e19d80b97d4d8cc208c1) |
| 21 | |
| 22 | CVE: CVE-2018-15686 |
| 23 | Upstream-Status: Backport |
| 24 | |
| 25 | Signed-off-by: Chen Qi <Qi.Chen@windriver.com> |
| 26 | --- |
| 27 | src/core/job.c | 19 +++++++++++-------- |
| 28 | src/core/manager.c | 44 ++++++++++++++++++++------------------------ |
| 29 | src/core/unit.c | 34 ++++++++++++++++++---------------- |
| 30 | src/core/unit.h | 2 +- |
| 31 | 4 files changed, 50 insertions(+), 49 deletions(-) |
| 32 | |
| 33 | diff --git a/src/core/job.c b/src/core/job.c |
| 34 | index 734756b..8552ffb 100644 |
| 35 | --- a/src/core/job.c |
| 36 | +++ b/src/core/job.c |
| 37 | @@ -10,6 +10,7 @@ |
| 38 | #include "dbus-job.h" |
| 39 | #include "dbus.h" |
| 40 | #include "escape.h" |
| 41 | +#include "fileio.h" |
| 42 | #include "job.h" |
| 43 | #include "log.h" |
| 44 | #include "macro.h" |
| 45 | @@ -1091,24 +1092,26 @@ int job_serialize(Job *j, FILE *f) { |
| 46 | } |
| 47 | |
| 48 | int job_deserialize(Job *j, FILE *f) { |
| 49 | + int r; |
| 50 | + |
| 51 | assert(j); |
| 52 | assert(f); |
| 53 | |
| 54 | for (;;) { |
| 55 | - char line[LINE_MAX], *l, *v; |
| 56 | + _cleanup_free_ char *line = NULL; |
| 57 | + char *l, *v; |
| 58 | size_t k; |
| 59 | |
| 60 | - if (!fgets(line, sizeof(line), f)) { |
| 61 | - if (feof(f)) |
| 62 | - return 0; |
| 63 | - return -errno; |
| 64 | - } |
| 65 | + r = read_line(f, LONG_LINE_MAX, &line); |
| 66 | + if (r < 0) |
| 67 | + return log_error_errno(r, "Failed to read serialization line: %m"); |
| 68 | + if (r == 0) |
| 69 | + return 0; |
| 70 | |
| 71 | - char_array_0(line); |
| 72 | l = strstrip(line); |
| 73 | |
| 74 | /* End marker */ |
| 75 | - if (l[0] == 0) |
| 76 | + if (isempty(l)) |
| 77 | return 0; |
| 78 | |
| 79 | k = strcspn(l, "="); |
| 80 | diff --git a/src/core/manager.c b/src/core/manager.c |
| 81 | index 3a7f0c4..a5780c9 100644 |
| 82 | --- a/src/core/manager.c |
| 83 | +++ b/src/core/manager.c |
| 84 | @@ -3171,22 +3171,19 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { |
| 85 | m->n_reloading++; |
| 86 | |
| 87 | for (;;) { |
| 88 | - char line[LINE_MAX]; |
| 89 | + _cleanup_free_ char *line = NULL; |
| 90 | const char *val, *l; |
| 91 | |
| 92 | - if (!fgets(line, sizeof(line), f)) { |
| 93 | - if (feof(f)) |
| 94 | - r = 0; |
| 95 | - else |
| 96 | - r = -errno; |
| 97 | - |
| 98 | + r = read_line(f, LONG_LINE_MAX, &line); |
| 99 | + if (r < 0) { |
| 100 | + log_error_errno(r, "Failed to read serialization line: %m"); |
| 101 | goto finish; |
| 102 | } |
| 103 | + if (r == 0) |
| 104 | + break; |
| 105 | |
| 106 | - char_array_0(line); |
| 107 | l = strstrip(line); |
| 108 | - |
| 109 | - if (l[0] == 0) |
| 110 | + if (isempty(l)) /* end marker */ |
| 111 | break; |
| 112 | |
| 113 | if ((val = startswith(l, "current-job-id="))) { |
| 114 | @@ -3353,29 +3350,31 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { |
| 115 | } |
| 116 | |
| 117 | for (;;) { |
| 118 | - Unit *u; |
| 119 | - char name[UNIT_NAME_MAX+2]; |
| 120 | + _cleanup_free_ char *line = NULL; |
| 121 | const char* unit_name; |
| 122 | + Unit *u; |
| 123 | |
| 124 | /* Start marker */ |
| 125 | - if (!fgets(name, sizeof(name), f)) { |
| 126 | - if (feof(f)) |
| 127 | - r = 0; |
| 128 | - else |
| 129 | - r = -errno; |
| 130 | - |
| 131 | + r = read_line(f, LONG_LINE_MAX, &line); |
| 132 | + if (r < 0) { |
| 133 | + log_error_errno(r, "Failed to read serialization line: %m"); |
| 134 | goto finish; |
| 135 | } |
| 136 | + if (r == 0) |
| 137 | + break; |
| 138 | |
| 139 | - char_array_0(name); |
| 140 | - unit_name = strstrip(name); |
| 141 | + unit_name = strstrip(line); |
| 142 | |
| 143 | r = manager_load_unit(m, unit_name, NULL, NULL, &u); |
| 144 | if (r < 0) { |
| 145 | log_notice_errno(r, "Failed to load unit \"%s\", skipping deserialization: %m", unit_name); |
| 146 | if (r == -ENOMEM) |
| 147 | goto finish; |
| 148 | - unit_deserialize_skip(f); |
| 149 | + |
| 150 | + r = unit_deserialize_skip(f); |
| 151 | + if (r < 0) |
| 152 | + goto finish; |
| 153 | + |
| 154 | continue; |
| 155 | } |
| 156 | |
| 157 | @@ -3388,9 +3387,6 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) { |
| 158 | } |
| 159 | |
| 160 | finish: |
| 161 | - if (ferror(f)) |
| 162 | - r = -EIO; |
| 163 | - |
| 164 | assert(m->n_reloading > 0); |
| 165 | m->n_reloading--; |
| 166 | |
| 167 | diff --git a/src/core/unit.c b/src/core/unit.c |
| 168 | index 7da963a..e98c9c4 100644 |
| 169 | --- a/src/core/unit.c |
| 170 | +++ b/src/core/unit.c |
| 171 | @@ -3380,21 +3380,19 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { |
| 172 | assert(fds); |
| 173 | |
| 174 | for (;;) { |
| 175 | - char line[LINE_MAX], *l, *v; |
| 176 | + _cleanup_free_ char *line = NULL; |
| 177 | CGroupIPAccountingMetric m; |
| 178 | + char *l, *v; |
| 179 | size_t k; |
| 180 | |
| 181 | - if (!fgets(line, sizeof(line), f)) { |
| 182 | - if (feof(f)) |
| 183 | - return 0; |
| 184 | - return -errno; |
| 185 | - } |
| 186 | + r = read_line(f, LONG_LINE_MAX, &line); |
| 187 | + if (r < 0) |
| 188 | + return log_error_errno(r, "Failed to read serialization line: %m"); |
| 189 | + if (r == 0) /* eof */ |
| 190 | + break; |
| 191 | |
| 192 | - char_array_0(line); |
| 193 | l = strstrip(line); |
| 194 | - |
| 195 | - /* End marker */ |
| 196 | - if (isempty(l)) |
| 197 | + if (isempty(l)) /* End marker */ |
| 198 | break; |
| 199 | |
| 200 | k = strcspn(l, "="); |
| 201 | @@ -3671,23 +3669,27 @@ int unit_deserialize(Unit *u, FILE *f, FDSet *fds) { |
| 202 | return 0; |
| 203 | } |
| 204 | |
| 205 | -void unit_deserialize_skip(FILE *f) { |
| 206 | +int unit_deserialize_skip(FILE *f) { |
| 207 | + int r; |
| 208 | assert(f); |
| 209 | |
| 210 | /* Skip serialized data for this unit. We don't know what it is. */ |
| 211 | |
| 212 | for (;;) { |
| 213 | - char line[LINE_MAX], *l; |
| 214 | + _cleanup_free_ char *line = NULL; |
| 215 | + char *l; |
| 216 | |
| 217 | - if (!fgets(line, sizeof line, f)) |
| 218 | - return; |
| 219 | + r = read_line(f, LONG_LINE_MAX, &line); |
| 220 | + if (r < 0) |
| 221 | + return log_error_errno(r, "Failed to read serialization line: %m"); |
| 222 | + if (r == 0) |
| 223 | + return 0; |
| 224 | |
| 225 | - char_array_0(line); |
| 226 | l = strstrip(line); |
| 227 | |
| 228 | /* End marker */ |
| 229 | if (isempty(l)) |
| 230 | - return; |
| 231 | + return 1; |
| 232 | } |
| 233 | } |
| 234 | |
| 235 | diff --git a/src/core/unit.h b/src/core/unit.h |
| 236 | index 06321bb..51c7aaa 100644 |
| 237 | --- a/src/core/unit.h |
| 238 | +++ b/src/core/unit.h |
| 239 | @@ -684,7 +684,7 @@ bool unit_can_serialize(Unit *u) _pure_; |
| 240 | |
| 241 | int unit_serialize(Unit *u, FILE *f, FDSet *fds, bool serialize_jobs); |
| 242 | int unit_deserialize(Unit *u, FILE *f, FDSet *fds); |
| 243 | -void unit_deserialize_skip(FILE *f); |
| 244 | +int unit_deserialize_skip(FILE *f); |
| 245 | |
| 246 | int unit_serialize_item(Unit *u, FILE *f, const char *key, const char *value); |
| 247 | int unit_serialize_item_escaped(Unit *u, FILE *f, const char *key, const char *value); |
| 248 | -- |
| 249 | 2.7.4 |
| 250 | |