dhcp-option: refuse control and non-UTF8 characters in string option

We oftem save parsed DHCP options into a file, or expose them
through DBus or Varlink. In such case, control characters or non-UTF8
characters may cause many kind of unexpected errors. In general, a DHCP
message that have string options with spurious characters is mostly
malformed or broken. Let's refuse them.

This also makes dhcp_option_parse_string() do not free 'ret' argument,
to follow our usual coding style. So, callers now need to free the
pre-exisitng string if necessary.

Fixes #31708.

diff --git a/src/libsystemd-network/dhcp-option.c b/src/libsystemd-network/dhcp-option.c
index 4025dd36325e2f8b02be52c1878654c09a50404d..4a6fa462f12f88963da60972134af3d614eac662 100644 (file)
--- a/src/libsystemd-network/dhcp-option.c
+++ b/src/libsystemd-network/dhcp-option.c
@@ -396,27 +396,27 @@ int dhcp_option_parse(DHCPMessage *message, size_t len, dhcp_option_callback_t c
 }
 
 int dhcp_option_parse_string(const uint8_t *option, size_t len, char **ret) {
+        _cleanup_free_ char *string = NULL;
         int r;
 
         assert(option);
         assert(ret);
 
-        if (len <= 0)
-                *ret = mfree(*ret);
-        else {
-                char *string;
+        if (len <= 0) {
+                *ret = NULL;
+                return 0;
+        }
 
-                /*
-                 * One trailing NUL byte is OK, we don't mind. See:
-                 * https://github.com/systemd/systemd/issues/1337
-                 */
-                r = make_cstring((const char *) option, len, MAKE_CSTRING_ALLOW_TRAILING_NUL, &string);
-                if (r < 0)
-                        return r;
+        /* One trailing NUL byte is OK, we don't mind. See:
+         * https://github.com/systemd/systemd/issues/1337 */
+        r = make_cstring((const char *) option, len, MAKE_CSTRING_ALLOW_TRAILING_NUL, &string);
+        if (r < 0)
+                return r;
 
-                free_and_replace(*ret, string);
-        }
+        if (!string_is_safe(string) || !utf8_is_valid(string))
+                return -EINVAL;
 
+        *ret = TAKE_PTR(string);
         return 0;
 }
 

diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c
index 401e70823a93237926a72fc7b765dfb89e0061f5..37f4b3b2c94f75747cb45c4466e566ecee14ff26 100644 (file)
--- a/src/libsystemd-network/sd-dhcp-lease.c
+++ b/src/libsystemd-network/sd-dhcp-lease.c
@@ -832,12 +832,16 @@ int dhcp_lease_parse_options(uint8_t code, uint8_t len, const void *option, void
 
                 break;
 
-        case SD_DHCP_OPTION_ROOT_PATH:
-                r = dhcp_option_parse_string(option, len, &lease->root_path);
+        case SD_DHCP_OPTION_ROOT_PATH: {
+                _cleanup_free_ char *p = NULL;
+
+                r = dhcp_option_parse_string(option, len, &p);
                 if (r < 0)
                         log_debug_errno(r, "Failed to parse root path, ignoring: %m");
-                break;
 
+                free_and_replace(lease->root_path, p);
+                break;
+        }
         case SD_DHCP_OPTION_RENEWAL_TIME:
                 r = lease_parse_be32_seconds(option, len, /* max_as_infinity = */ true, &lease->t1);
                 if (r < 0)