From fa3357b9e8d9d7a486902d0b6d4b4015fc10aac0 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Tue, 12 Mar 2024 01:32:03 +0900
Subject: [PATCH] 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.
---
 src/libsystemd-network/dhcp-option.c   | 28 +++++++++++++-------------
 src/libsystemd-network/sd-dhcp-lease.c | 10 ++++++---
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/libsystemd-network/dhcp-option.c b/src/libsystemd-network/dhcp-option.c
index 4025dd3632..4a6fa462f1 100644
--- 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;
-
-                /*
-                 * 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 (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;
+
+        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 401e70823a..37f4b3b2c9 100644
--- 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)