From a9d7abbc5023b51a6e4e8a5e2644fd1a2a20c838 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 7 Jul 2025 09:32:16 +0200
Subject: [PATCH] dhcp: fix parsing of the search list option

The DHCP search list option (119) can use the "message compression"
algorithm specified in RFC 1035 section 4.1.4 to reduce the size of
the message in presence of subdomains that appear multiple times.

When using the compression a label starts with:

    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
    | 1  1|                OFFSET                   |
    +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+

where the offset points to a previous domain.

Previously, the parsing code was taking the lower 6 bits of the first
byte, shifting them left 16 bits, and adding the next byte. Instead,
the shift should be of 8 bits.

The effect of this bug was that when the offset was greater than 255,
it was incorrectly parsed as a number larger than the message size,
and the parsing failed.

Note that while a single DHCP option can be at most 255 bytes, a DHCP
message can contain multiple instances of the same option. The
receiver must concatenate all the occurrences according to RFC 3396
and parse the resulting buffer.

Fixes: 6adade6f21d5 ('dhcp: add nettools dhcp4 client')
---
 src/core/dhcp/nm-dhcp-utils.c         |  2 +-
 src/core/dhcp/tests/test-dhcp-utils.c | 70 +++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/core/dhcp/nm-dhcp-utils.c b/src/core/dhcp/nm-dhcp-utils.c
index 15293fa38..949d87207 100644
--- a/src/core/dhcp/nm-dhcp-utils.c
+++ b/src/core/dhcp/nm-dhcp-utils.c
@@ -1224,7 +1224,7 @@ lease_option_print_domain_name(const uint8_t  *cache,
         }
         case 0xC0: /* back pointer */
         {
-            size_t offset = (c & 0x3F) << 16;
+            size_t offset = (c & 0x3F) << 8;
 
             /*
              * The offset is given as two bytes (in big endian), where the
diff --git a/src/core/dhcp/tests/test-dhcp-utils.c b/src/core/dhcp/tests/test-dhcp-utils.c
index b81523e19..de1be6538 100644
--- a/src/core/dhcp/tests/test-dhcp-utils.c
+++ b/src/core/dhcp/tests/test-dhcp-utils.c
@@ -239,6 +239,76 @@ test_parse_search_list(void)
     g_assert_cmpint(g_strv_length(domains), ==, 1);
     g_assert_cmpstr(domains[0], ==, "okay");
     g_strfreev(domains);
+
+    /* Test that the message compression works when the offset uses both bytes */
+    data = (guint8[]) {
+        /* clang-format off */
+         /* offset 0 */
+         0x3e,
+        'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
+        'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
+        'a','a','a','a','a','a','a','a','a','a','a','a','a','a','a','a',
+        'a','a','a','a','a','a','a','a','a','a','a','a','a','a',
+        0x00,
+        /* offset 0x40 */
+        0x3e,
+        'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
+        'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
+        'b','b','b','b','b','b','b','b','b','b','b','b','b','b','b','b',
+        'b','b','b','b','b','b','b','b','b','b','b','b','b','b',
+        0x00,
+        /* offset 0x80 */
+        0x3e,
+        'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
+        'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
+        'c','c','c','c','c','c','c','c','c','c','c','c','c','c','c','c',
+        'c','c','c','c','c','c','c','c','c','c','c','c','c','c',
+        0x00,
+        /* offset 0xc0 */
+        0x3e,
+        'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
+        'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
+        'd','d','d','d','d','d','d','d','d','d','d','d','d','d','d','d',
+        'd','d','d','d','d','d','d','d','d','d','d','d','d','d',
+        0x00,
+        /* offset 0x100 */
+        0x3e,
+        'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
+        'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
+        'e','e','e','e','e','e','e','e','e','e','e','e','e','e','e','e',
+        'e','e','e','e','e','e','e','e','e','e','e','e','e','e',
+        0x00,
+        /* offset 0x140 */
+        0x06, 'f','o','o','b','a','r', 0x03, 'c', 'o', 'm', 0x00,
+        0x04, 't', 'e', 's', 't', 0xc1, 0x40, /* back pointer to offset 0x140*/
+        /* clang-format on */
+    };
+
+    domains = nm_dhcp_lease_data_parse_search_list(data,
+                                                   0x153,
+                                                   "eth0",
+                                                   AF_INET,
+                                                   NM_DHCP_OPTION_DHCP4_DOMAIN_SEARCH_LIST);
+    g_assert(domains);
+    g_assert_cmpint(g_strv_length(domains), ==, 7);
+    g_assert_cmpstr(domains[0],
+                    ==,
+                    "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
+    g_assert_cmpstr(domains[1],
+                    ==,
+                    "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb");
+    g_assert_cmpstr(domains[2],
+                    ==,
+                    "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc");
+    g_assert_cmpstr(domains[3],
+                    ==,
+                    "dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd");
+    g_assert_cmpstr(domains[4],
+                    ==,
+                    "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee");
+    g_assert_cmpstr(domains[5], ==, "foobar.com");
+    g_assert_cmpstr(domains[6], ==, "test.foobar.com");
+    g_strfreev(domains);
 }
 
 static void