F-9758: validate FDT layout in fdt_check_header, use it in fdt_get_string

fdt_get_string() bounded stroffset against size_dt_strings but
formed the string-table pointer from off_dt_strings without ever
validating either header field against totalsize; a DTB declaring a
large off_dt_strings with a small size_dt_strings made every
property lookup (fdt_getprop -> fdt_get_string) scan far outside
the blob.

Validate the structural layout in fdt_check_header() for finalized
(FDT_MAGIC) blobs: the reservation map, structure block and string
table must sit inside the blob and not overlap, checked in 64-bit
so the size fields cannot wrap. fdt_get_string() now requires a
valid header before forming the pointer. The SW_MAGIC (in-progress
edit) state keeps its existing check, since its layout is different.

Test fixtures are adjusted to the validated layout: the two
pre-existing fdt_get_string fixtures now set the header fields the
lookup relies on, the compatible-test builder sets the magic word
and points the reservation map at the canonical empty list right
after the header (it pointed into the string table before).

src/fdt.c

@@ -462,10 +462,24 @@ static int fdt_subnode_offset_namelen(const void *fdt, int offset,
 int fdt_check_header(const void *fdt)
 {
     if (fdt_magic(fdt) == FDT_MAGIC) {
+        uint32_t off_rsv = fdt_off_mem_rsvmap(fdt);
+        uint32_t off_struct = fdt_off_dt_struct(fdt);
+        uint32_t size_struct = fdt_size_dt_struct(fdt);
+        uint32_t off_strings = fdt_off_dt_strings(fdt);
+        uint32_t size_strings = fdt_size_dt_strings(fdt);
+
         if (fdt_version(fdt) < FDT_FIRST_SUPPORTED_VERSION)
             return -FDT_ERR_BADVERSION;
         if (fdt_last_comp_version(fdt) > FDT_LAST_SUPPORTED_VERSION)
             return -FDT_ERR_BADVERSION;
+        /* The three structural areas must sit inside the blob and not
+         * overlap: reservation map, structure block and string table,
+         * in that order. The additions are made in 64-bit so the size
+         * fields cannot wrap around the comparison. */
+        if (off_rsv > off_struct
+            || (uint64_t)off_struct + size_struct > off_strings
+            || (uint64_t)off_strings + size_strings > fdt_totalsize(fdt))
+            return -FDT_ERR_BADSTRUCTURE;
     }
     else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
         if (fdt_size_dt_struct(fdt) == 0)
@@ -579,6 +593,17 @@ const char* fdt_get_string(const void *fdt, int stroffset, int *lenp)
     uint32_t strsize = fdt_size_dt_strings(fdt);
     const char *s;
     const char *end;
+    int err;
+
+    /* off_dt_strings/size_dt_strings are attacker-influenceable header
+     * fields; validate the layout against totalsize before forming the
+     * string-table pointer. */
+    err = fdt_check_header(fdt);
+    if (err != 0) {
+        if (lenp)
+            *lenp = err;
+        return NULL;
+    }
 
     if ((stroffset < 0) || ((uint32_t)stroffset >= strsize)) {
         if (lenp)

tools/unit-tests/unit-fdt.c

@@ -43,8 +43,12 @@ START_TEST(test_fdt_get_string_rejects_out_of_range_offset)
     const char *s;
 
     memset(&blob, 0, sizeof(blob));
+    fdt_set_totalsize(&blob, sizeof(blob.hdr) + sizeof(blob.strings));
     fdt_set_off_dt_strings(&blob, sizeof(blob.hdr));
     fdt_set_size_dt_strings(&blob, sizeof(blob.strings));
+    fdt_set_magic(&blob, FDT_MAGIC);
+    fdt_set_version(&blob, 17);
+    fdt_set_last_comp_version(&blob, 16);
     memcpy(blob.strings, "chosen", sizeof("chosen"));
     blob.after[0] = 'X';
     blob.after[1] = '\0';
@@ -66,8 +70,12 @@ START_TEST(test_fdt_get_string_returns_string_with_valid_offset)
     const char *s;
 
     memset(&blob, 0, sizeof(blob));
+    fdt_set_totalsize(&blob, sizeof(blob.hdr) + sizeof(blob.strings));
     fdt_set_off_dt_strings(&blob, sizeof(blob.hdr));
     fdt_set_size_dt_strings(&blob, sizeof(blob.strings));
+    fdt_set_magic(&blob, FDT_MAGIC);
+    fdt_set_version(&blob, 17);
+    fdt_set_last_comp_version(&blob, 16);
     memcpy(blob.strings, "serial\0console\0", 15);
 
     s = fdt_get_string(&blob, 7, &len);
@@ -218,6 +226,7 @@ static void build_compat_fdt(uint8_t *buf, size_t size,
     memset(buf, 0, size);
 
     hdr = (struct fdt_header *)buf;
+    hdr->magic = fdt32_to_cpu(FDT_MAGIC);
     fdt_set_totalsize(hdr, 0x100);
     fdt_set_off_dt_struct(hdr, struct_off);
     fdt_set_off_dt_strings(hdr, strings_off);
@@ -296,6 +305,41 @@ START_TEST(test_fdt_compatible_prefix_entry_no_match)
     off = fdt_node_offset_by_compatible(buf, -1, "abc");
     ck_assert_int_lt(off, 0);
 }
+END_TEST
+
+/* A finalized DTB whose string table lies past the declared end of
+ * the blob must be rejected: before the fix fdt_check_header()
+ * validated only magic and version, and fdt_get_string() formed the
+ * string-table pointer from the unvalidated header fields. */
+START_TEST(test_fdt_check_header_rejects_unbounded_string_area)
+{
+    static uint8_t buf[0x100];
+    int len = 0;
+    const char *s;
+
+    build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4);
+    /* push the string table past the declared end of the blob */
+    fdt_set_off_dt_strings((struct fdt_header *)buf, 0x100);
+
+    ck_assert_int_eq(fdt_check_header(buf), -FDT_ERR_BADSTRUCTURE);
+
+    s = fdt_get_string(buf, 0, &len);
+    ck_assert_ptr_null(s);
+    ck_assert_int_lt(len, 0);
+}
+END_TEST
+
+/* The structure block must not overlap the string table. */
+START_TEST(test_fdt_check_header_rejects_overlapping_areas)
+{
+    static uint8_t buf[0x100];
+
+    build_compat_fdt(buf, sizeof(buf), (const uint8_t *)"abc\0", 4);
+    fdt_set_size_dt_struct((struct fdt_header *)buf, 0x100);
+
+    ck_assert_int_eq(fdt_check_header(buf), -FDT_ERR_BADSTRUCTURE);
+}
+END_TEST
 
 static Suite *fdt_suite(void)
 {
@@ -313,6 +357,8 @@ static Suite *fdt_suite(void)
     tcase_add_test(tc, test_fdt_compatible_terminated_exact_len_match);
     tcase_add_test(tc, test_fdt_compatible_multi_string_list_match);
     tcase_add_test(tc, test_fdt_compatible_prefix_entry_no_match);
+    tcase_add_test(tc, test_fdt_check_header_rejects_unbounded_string_area);
+    tcase_add_test(tc, test_fdt_check_header_rejects_overlapping_areas);
     suite_add_tcase(s, tc);
 
     tcase_set_timeout(tc_dos, 5);

tools/unit-tests/unit-t10xx-dts-memac.c

@@ -261,7 +261,9 @@ static void dtb_finalize(struct dtb *d)
     hdr->totalsize = cpu_to_fdt32(0x2000);
     hdr->off_dt_struct = cpu_to_fdt32(d->struct_off);
     hdr->off_dt_strings = cpu_to_fdt32(d->strings_off);
-    hdr->off_mem_rsvmap = cpu_to_fdt32(d->strings_off); /* no reservations */
+    hdr->off_mem_rsvmap = cpu_to_fdt32(0x28);
+    /* the 8 bytes after the 40-byte header are zero: an empty
+     * reservation list at the canonical spot */
     hdr->version = cpu_to_fdt32(17);
     hdr->last_comp_version = cpu_to_fdt32(16);
     hdr->boot_cpuid_phys = cpu_to_fdt32(0);