From: Lennart Poettering <lennart@poettering.net>
Date: Sat, 15 Dec 2018 11:25:32 +0000 (+0100)
Subject: fileio: when reading a full file into memory, refuse inner NUL bytes
X-Git-Tag: v240~47
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2d78717b09d05b9a9164cb118fb89924570a5e61;p=thirdparty%2Fsystemd.git

fileio: when reading a full file into memory, refuse inner NUL bytes

Just some extra care to avoid any ambiguities in what we read.
---

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index d434cb4d2c9..b22fcd03027 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -266,16 +266,20 @@ int verify_file(const char *fn, const char *blob, bool accept_extra_nl) {
         return 1;
 }
 
-int read_full_stream(FILE *f, char **contents, size_t *size) {
+int read_full_stream(
+                FILE *f,
+                char **ret_contents,
+                size_t *ret_size) {
+
         _cleanup_free_ char *buf = NULL;
         struct stat st;
         size_t n, l;
         int fd;
 
         assert(f);
-        assert(contents);
+        assert(ret_contents);
 
-        n = LINE_MAX;
+        n = LINE_MAX; /* Start size */
 
         fd = fileno(f);
         if (fd >= 0) { /* If the FILE* object is backed by an fd (as opposed to memory or such, see fmemopen(), let's
@@ -331,11 +335,20 @@ int read_full_stream(FILE *f, char **contents, size_t *size) {
                 n = MIN(n * 2, READ_FULL_BYTES_MAX);
         }
 
+        if (!ret_size) {
+                /* Safety check: if the caller doesn't want to know the size of what we just read it will rely on the
+                 * trailing NUL byte. But if there's an embedded NUL byte, then we should refuse operation as otherwise
+                 * there'd be ambiguity about what we just read. */
+
+                if (memchr(buf, 0, l))
+                        return -EBADMSG;
+        }
+
         buf[l] = 0;
-        *contents = TAKE_PTR(buf);
+        *ret_contents = TAKE_PTR(buf);
 
-        if (size)
-                *size = l;
+        if (ret_size)
+                *ret_size = l;
 
         return 0;
 }
diff --git a/src/test/test-json.c b/src/test/test-json.c
index d3ece0bc0ba..5aa4d19dbec 100644
--- a/src/test/test-json.c
+++ b/src/test/test-json.c
@@ -358,7 +358,7 @@ static void test_source(void) {
                "%s"
                "--- original end ---\n", data);
 
-        assert_se(f = fmemopen((void*) data, sizeof(data), "r"));
+        assert_se(f = fmemopen((void*) data, strlen(data), "r"));
 
         assert_se(json_parse_file(f, "waldo", &v, NULL, NULL) >= 0);
 
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index 13c289270de..f5578f9fc27 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -530,7 +530,7 @@ static void test_load_env_file_1(void) {
 
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
-        assert_se(write(fd, env_file_1, sizeof(env_file_1)) == sizeof(env_file_1));
+        assert_se(write(fd, env_file_1, strlen(env_file_1)) == strlen(env_file_1));
 
         r = load_env_file(NULL, name, &data);
         assert_se(r == 0);
@@ -552,7 +552,7 @@ static void test_load_env_file_2(void) {
 
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
-        assert_se(write(fd, env_file_2, sizeof(env_file_2)) == sizeof(env_file_2));
+        assert_se(write(fd, env_file_2, strlen(env_file_2)) == strlen(env_file_2));
 
         r = load_env_file(NULL, name, &data);
         assert_se(r == 0);
@@ -569,7 +569,7 @@ static void test_load_env_file_3(void) {
 
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
-        assert_se(write(fd, env_file_3, sizeof(env_file_3)) == sizeof(env_file_3));
+        assert_se(write(fd, env_file_3, strlen(env_file_3)) == strlen(env_file_3));
 
         r = load_env_file(NULL, name, &data);
         assert_se(r == 0);
@@ -584,7 +584,7 @@ static void test_load_env_file_4(void) {
 
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
-        assert_se(write(fd, env_file_4, sizeof(env_file_4)) == sizeof(env_file_4));
+        assert_se(write(fd, env_file_4, strlen(env_file_4)) == strlen(env_file_4));
 
         r = load_env_file(NULL, name, &data);
         assert_se(r == 0);
@@ -603,7 +603,7 @@ static void test_load_env_file_5(void) {
 
         fd = mkostemp_safe(name);
         assert_se(fd >= 0);
-        assert_se(write(fd, env_file_5, sizeof(env_file_5)) == sizeof(env_file_5));
+        assert_se(write(fd, env_file_5, strlen(env_file_5)) == strlen(env_file_5));
 
         r = load_env_file(NULL, name, &data);
         assert_se(r == 0);