From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 20 Jan 2023 14:36:09 +0000 (+0100)
Subject: string-util: add common implementation of function that converts sized character... 
X-Git-Tag: v253-rc1~44
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7153213e406815ae0083789c211d8b77c79588d5;p=thirdparty%2Fsystemd.git

string-util: add common implementation of function that converts sized character buffers to NUL terminated C strings
---

diff --git a/src/basic/string-util.c b/src/basic/string-util.c
index 577afd37f42..ad8c9863bdd 100644
--- a/src/basic/string-util.c
+++ b/src/basic/string-util.c
@@ -1187,6 +1187,49 @@ char *string_replace_char(char *str, char old_char, char new_char) {
         return str;
 }
 
+int make_cstring(const char *s, size_t n, MakeCStringMode mode, char **ret) {
+        char *b;
+
+        assert(s || n == 0);
+        assert(mode >= 0);
+        assert(mode < _MAKE_CSTRING_MODE_MAX);
+
+        /* Converts a sized character buffer into a NUL-terminated NUL string, refusing if there are embedded
+         * NUL bytes. Whether to expect a trailing NUL byte can be specified via 'mode' */
+
+        if (n == 0) {
+                if (mode == MAKE_CSTRING_REQUIRE_TRAILING_NUL)
+                        return -EINVAL;
+
+                if (!ret)
+                        return 0;
+
+                b = new0(char, 1);
+        } else {
+                const char *nul;
+
+                nul = memchr(s, 0, n);
+                if (nul) {
+                        if (nul < s + n - 1 || /* embedded NUL? */
+                            mode == MAKE_CSTRING_REFUSE_TRAILING_NUL)
+                                return -EINVAL;
+
+                        n--;
+                } else if (mode == MAKE_CSTRING_REQUIRE_TRAILING_NUL)
+                        return -EINVAL;
+
+                if (!ret)
+                        return 0;
+
+                b = memdup_suffix0(s, n);
+        }
+        if (!b)
+                return -ENOMEM;
+
+        *ret = b;
+        return 0;
+}
+
 size_t strspn_from_end(const char *str, const char *accept) {
         size_t n = 0;
 
diff --git a/src/basic/string-util.h b/src/basic/string-util.h
index 684b7d5c118..e0a47a21a97 100644
--- a/src/basic/string-util.h
+++ b/src/basic/string-util.h
@@ -239,6 +239,16 @@ bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok);
 
 char *string_replace_char(char *str, char old_char, char new_char);
 
+typedef enum MakeCStringMode {
+        MAKE_CSTRING_REFUSE_TRAILING_NUL,
+        MAKE_CSTRING_ALLOW_TRAILING_NUL,
+        MAKE_CSTRING_REQUIRE_TRAILING_NUL,
+        _MAKE_CSTRING_MODE_MAX,
+        _MAKE_CSTRING_MODE_INVALID = -1,
+} MakeCStringMode;
+
+int make_cstring(const char *s, size_t n, MakeCStringMode mode, char **ret);
+
 size_t strspn_from_end(const char *str, const char *accept);
 
 char *strdupspn(const char *a, const char *accept);
diff --git a/src/creds/creds.c b/src/creds/creds.c
index 71bf355b383..3eb94cd36a2 100644
--- a/src/creds/creds.c
+++ b/src/creds/creds.c
@@ -326,16 +326,12 @@ static int write_blob(FILE *f, const void *data, size_t size) {
 
         if (arg_transcode == TRANSCODE_OFF &&
             arg_json_format_flags != JSON_FORMAT_OFF) {
-
                 _cleanup_(erase_and_freep) char *suffixed = NULL;
                 _cleanup_(json_variant_unrefp) JsonVariant *v = NULL;
 
-                if (memchr(data, 0, size))
-                        return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Credential data contains embedded NUL, can't parse as JSON.");
-
-                suffixed = memdup_suffix0(data, size);
-                if (!suffixed)
-                        return log_oom();
+                r = make_cstring(data, size, MAKE_CSTRING_REFUSE_TRAILING_NUL, &suffixed);
+                if (r < 0)
+                        return log_error_errno(r, "Unable to convert binary string to C string: %m");
 
                 r = json_parse(suffixed, JSON_PARSE_SENSITIVE, &v, NULL, NULL);
                 if (r < 0)
diff --git a/src/cryptsetup/cryptsetup-tokens/cryptsetup-token-util.c b/src/cryptsetup/cryptsetup-tokens/cryptsetup-token-util.c
index e305d8ba795..4e3090bc929 100644
--- a/src/cryptsetup/cryptsetup-tokens/cryptsetup-token-util.c
+++ b/src/cryptsetup/cryptsetup-tokens/cryptsetup-token-util.c
@@ -58,27 +58,13 @@ int crypt_dump_hex_string(const char *hex_str, char **ret_dump_str) {
 }
 
 int crypt_normalize_pin(const void *pin, size_t pin_size, char **ret_pin_string) {
-
-        _cleanup_free_ char *pin_string = NULL;
-
-        assert(pin || !pin_size);
+        assert(pin || pin_size == 0);
         assert(ret_pin_string);
 
-        if (!pin) {
+        if (pin_size == 0) {
                 *ret_pin_string = NULL;
                 return 0;
         }
 
-        /* Refuse embedded NULL bytes, but allow trailing NULL */
-        if (memchr(pin, 0, pin_size - 1))
-                return -EINVAL;
-
-        /* Enforce trailing NULL byte if missing */
-        pin_string = memdup_suffix0(pin, pin_size);
-        if (!pin_string)
-                return -ENOMEM;
-
-        *ret_pin_string = TAKE_PTR(pin_string);
-
-        return 0;
+        return make_cstring(pin, pin_size, MAKE_CSTRING_ALLOW_TRAILING_NUL, ret_pin_string);
 }
diff --git a/src/libsystemd-network/dhcp-option.c b/src/libsystemd-network/dhcp-option.c
index 7f49da7c2d7..a52259c238a 100644
--- a/src/libsystemd-network/dhcp-option.c
+++ b/src/libsystemd-network/dhcp-option.c
@@ -279,6 +279,7 @@ static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overlo
         uint8_t code, len;
         const uint8_t *option;
         size_t offset = 0;
+        int r;
 
         while (offset < buflen) {
                 code = options[offset ++];
@@ -318,13 +319,9 @@ static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overlo
                         if (error_message) {
                                 _cleanup_free_ char *string = NULL;
 
-                                /* Accept a trailing NUL byte */
-                                if (memchr(option, 0, len - 1))
-                                        return -EINVAL;
-
-                                string = memdup_suffix0((const char *) option, len);
-                                if (!string)
-                                        return -ENOMEM;
+                                r = make_cstring((const char*) option, len, MAKE_CSTRING_ALLOW_TRAILING_NUL, &string);
+                                if (r < 0)
+                                        return r;
 
                                 if (!ascii_is_valid(string))
                                         return -EINVAL;
diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c
index b14ad57071d..02c6a6e0d79 100644
--- a/src/libsystemd-network/sd-dhcp-lease.c
+++ b/src/libsystemd-network/sd-dhcp-lease.c
@@ -376,6 +376,8 @@ static int lease_parse_be32(const uint8_t *option, size_t len, be32_t *ret) {
 }
 
 static int lease_parse_string(const uint8_t *option, size_t len, char **ret) {
+        int r;
+
         assert(option);
         assert(ret);
 
@@ -388,12 +390,9 @@ static int lease_parse_string(const uint8_t *option, size_t len, char **ret) {
                  * One trailing NUL byte is OK, we don't mind. See:
                  * https://github.com/systemd/systemd/issues/1337
                  */
-                if (memchr(option, 0, len - 1))
-                        return -EINVAL;
-
-                string = memdup_suffix0((const char *) option, len);
-                if (!string)
-                        return -ENOMEM;
+                r = make_cstring((const char*) option, len, MAKE_CSTRING_ALLOW_TRAILING_NUL, &string);
+                if (r < 0)
+                        return r;
 
                 free_and_replace(*ret, string);
         }
diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index f90e5974c4d..574a1a4be92 100644
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -1371,14 +1371,14 @@ int dns_packet_read_uint32(DnsPacket *p, uint32_t *ret, size_t *start) {
 }
 
 int dns_packet_read_string(DnsPacket *p, char **ret, size_t *start) {
-        assert(p);
-
         _cleanup_(rewind_dns_packet) DnsPacketRewinder rewinder = REWINDER_INIT(p);
+        _cleanup_free_ char *t = NULL;
         const void *d;
-        char *t;
         uint8_t c;
         int r;
 
+        assert(p);
+
         r = dns_packet_read_uint8(p, &c, NULL);
         if (r < 0)
                 return r;
@@ -1387,19 +1387,14 @@ int dns_packet_read_string(DnsPacket *p, char **ret, size_t *start) {
         if (r < 0)
                 return r;
 
-        if (memchr(d, 0, c))
-                return -EBADMSG;
-
-        t = memdup_suffix0(d, c);
-        if (!t)
-                return -ENOMEM;
+        r = make_cstring(d, c, MAKE_CSTRING_REFUSE_TRAILING_NUL, &t);
+        if (r < 0)
+                return r;
 
-        if (!utf8_is_valid(t)) {
-                free(t);
+        if (!utf8_is_valid(t))
                 return -EBADMSG;
-        }
 
-        *ret = t;
+        *ret = TAKE_PTR(t);
 
         if (start)
                 *start = rewinder.saved_rindex;
diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c
index b416e873afb..074440e99b9 100644
--- a/src/shared/creds-util.c
+++ b/src/shared/creds-util.c
@@ -1146,12 +1146,9 @@ int decrypt_credential_and_warn(
         if (le32toh(m->name_size) > 0) {
                 _cleanup_free_ char *embedded_name = NULL;
 
-                if (memchr(m->name, 0, le32toh(m->name_size)))
-                        return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Embedded credential name contains NUL byte, refusing.");
-
-                embedded_name = memdup_suffix0(m->name, le32toh(m->name_size));
-                if (!embedded_name)
-                        return log_oom();
+                r = make_cstring(m->name, le32toh(m->name_size), MAKE_CSTRING_REFUSE_TRAILING_NUL, &embedded_name);
+                if (r < 0)
+                        return log_error_errno(r, "Unable to convert embedded credential name to C string: %m");
 
                 if (!credential_name_valid(embedded_name))
                         return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Embedded credential name is not valid, refusing.");
diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c
index 4047139c269..b3ff7d65c1c 100644
--- a/src/test/test-string-util.c
+++ b/src/test/test-string-util.c
@@ -1168,4 +1168,54 @@ TEST(streq_skip_trailing_chars) {
         assert_se(!streq_skip_trailing_chars("", "f", NULL));
 }
 
+#define TEST_MAKE_CSTRING_ONE(x, ret, mode, expect)                     \
+        do {                                                            \
+                _cleanup_free_ char *b = NULL;                          \
+                assert_se(make_cstring((x), ELEMENTSOF(x), (mode), &b) == (ret)); \
+                assert_se(streq_ptr(b, (expect)));                      \
+        } while(false)
+
+TEST(make_cstring) {
+        static const char test1[] = "this is a test",
+                test2[] = "",
+                test3[] = "a",
+                test4[] = "aa\0aa",
+                test5[] = { 'b', 'b', 0, 'b' , 'b' },
+                test6[] = {},
+                test7[] = { 'x' },
+                test8[] = { 'x', 'y', 'z' };
+
+        TEST_MAKE_CSTRING_ONE(test1, -EINVAL, MAKE_CSTRING_REFUSE_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test1, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "this is a test");
+        TEST_MAKE_CSTRING_ONE(test1, 0, MAKE_CSTRING_REQUIRE_TRAILING_NUL, "this is a test");
+
+        TEST_MAKE_CSTRING_ONE(test2, -EINVAL, MAKE_CSTRING_REFUSE_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test2, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "");
+        TEST_MAKE_CSTRING_ONE(test2, 0, MAKE_CSTRING_REQUIRE_TRAILING_NUL, "");
+
+        TEST_MAKE_CSTRING_ONE(test3, -EINVAL, MAKE_CSTRING_REFUSE_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test3, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "a");
+        TEST_MAKE_CSTRING_ONE(test3, 0, MAKE_CSTRING_REQUIRE_TRAILING_NUL, "a");
+
+        TEST_MAKE_CSTRING_ONE(test4, -EINVAL, MAKE_CSTRING_REFUSE_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test4, -EINVAL, MAKE_CSTRING_ALLOW_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test4, -EINVAL, MAKE_CSTRING_REQUIRE_TRAILING_NUL, NULL);
+
+        TEST_MAKE_CSTRING_ONE(test5, -EINVAL, MAKE_CSTRING_REFUSE_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test5, -EINVAL, MAKE_CSTRING_ALLOW_TRAILING_NUL, NULL);
+        TEST_MAKE_CSTRING_ONE(test5, -EINVAL, MAKE_CSTRING_REQUIRE_TRAILING_NUL, NULL);
+
+        TEST_MAKE_CSTRING_ONE(test6, 0, MAKE_CSTRING_REFUSE_TRAILING_NUL, "");
+        TEST_MAKE_CSTRING_ONE(test6, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "");
+        TEST_MAKE_CSTRING_ONE(test6, -EINVAL, MAKE_CSTRING_REQUIRE_TRAILING_NUL, NULL);
+
+        TEST_MAKE_CSTRING_ONE(test7, 0, MAKE_CSTRING_REFUSE_TRAILING_NUL, "x");
+        TEST_MAKE_CSTRING_ONE(test7, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "x");
+        TEST_MAKE_CSTRING_ONE(test7, -EINVAL, MAKE_CSTRING_REQUIRE_TRAILING_NUL, NULL);
+
+        TEST_MAKE_CSTRING_ONE(test8, 0, MAKE_CSTRING_REFUSE_TRAILING_NUL, "xyz");
+        TEST_MAKE_CSTRING_ONE(test8, 0, MAKE_CSTRING_ALLOW_TRAILING_NUL, "xyz");
+        TEST_MAKE_CSTRING_ONE(test8, -EINVAL, MAKE_CSTRING_REQUIRE_TRAILING_NUL, NULL);
+}
+
 DEFINE_TEST_MAIN(LOG_DEBUG);