diff --git a/core/ed25519_verify.c b/core/ed25519_verify.c index d36cc7c..34aca77 100644 --- a/core/ed25519_verify.c +++ b/core/ed25519_verify.c @@ -289,6 +289,16 @@ static int point_is_identity(gf p[4]) return diff == 0; } +static void scalarbase(gf r[4], const uint8_t *s) +{ + gf q[4]; + fe_copy16(q[0], BX); + fe_copy16(q[1], BY); + fe_copy16(q[2], gf1); + fe_mul(q[3], BX, BY); + scalarmult(r, q, s); +} + /* Reject a public key outside the prime-order subgroup. * * Decoding a point is not enough. Ed25519 has eight points of low order, and @@ -304,51 +314,9 @@ static int point_is_identity(gf p[4]) * so there is no separate constant to transcribe wrongly: a mistyped L would * reject valid keys, and only in the field. * - * A arrives negated from unpackneg(). [L](-A) = -[L]A and the identity is its - * own negation, so neither condition is affected by the sign. - * - * Formulation taken from eBoot#57 by @muhammadburhandevv-hub, which reached - * this before I did and states both conditions in one expression. + * The key arrives negated from unpackneg(). [L](-A) = -[L]A and the identity + * is its own negation, so neither condition is affected by the sign. */ -static int key_has_prime_order(gf A[4]) -{ - uint8_t order_l[32]; - gf q[4], multiple[4]; - int i; - - for (i = 0; i < 32; i++) - order_l[i] = (uint8_t)ORDER_L[i]; - for (i = 0; i < 4; i++) - fe_copy16(q[i], A[i]); - - scalarmult(multiple, q, order_l); - return point_is_identity(multiple) && !point_is_identity(A); -} - -static void scalarbase(gf r[4], const uint8_t *s) -{ - gf q[4]; - fe_copy16(q[0], BX); - fe_copy16(q[1], BY); - fe_copy16(q[2], gf1); - fe_mul(q[3], BX, BY); - scalarmult(r, q, s); -} - -static int point_is_identity(gf p[4]) -{ - uint8_t encoded[32]; - point_pack(encoded, p); - - uint8_t diff = (uint8_t)(encoded[0] ^ 1U); - for (int i = 1; i < 32; i++) - diff |= encoded[i]; - return diff == 0; -} - -/* Public keys must be non-identity points in Ed25519's prime-order subgroup. - * Merely decoding a point is insufficient: an identity or torsion key can - * make the verification equation true without knowledge of a private key. */ static int public_key_is_valid_subgroup(gf public_key[4]) { uint8_t order_l[32]; diff --git a/core/fdt_loader.c b/core/fdt_loader.c index 1be0175..de90ac8 100644 --- a/core/fdt_loader.c +++ b/core/fdt_loader.c @@ -8,6 +8,7 @@ #include "eos_fdt_loader.h" #include "eos_hal.h" +#include #include /* Big-endian to host conversion */ @@ -26,15 +27,93 @@ static uint32_t fdt_read_u32(const uint8_t *ptr) return fdt32_to_cpu(val); } -int eos_fdt_validate(const void *fdt_blob) +/* Header fields, read the same way the struct block is read. The blob may + * be unaligned -- fuzz input, a buffer inside a larger message, a copy at an + * odd offset -- and dereferencing a cast fdt_header_t* is a misaligned load + * on strict-alignment targets, the same fault class this parser exists to + * avoid. One rule for the whole blob: every multi-byte read goes through + * memcpy. */ +static uint32_t fdt_hdr_u32(const void *blob, size_t field_off) +{ + return fdt_read_u32((const uint8_t *)blob + field_off); +} + +/* Does [off, off+len) sit inside a blob of totalsize bytes, without the + * addition wrapping? Every offset in the header is attacker-controlled. */ +static bool fdt_block_in_bounds(uint32_t off, uint32_t len, uint32_t totalsize) +{ + if (off < sizeof(fdt_header_t)) return false; + if (len > totalsize) return false; + return off <= totalsize - len; +} + +/* Does a device tree node name match one path component? + * + * Real node names carry a unit address -- `uart@40011000`, `serial@10000000` + * -- while a path is usually written without one. Exact string equality meant + * /soc/uart resolved only against trees whose peripherals happen to be named + * without an address, which is to say against hand-built test blobs and not + * against any actual DTB. (/ and /chosen were unaffected: by convention they + * carry no unit address, which is why the bootargs path this parser exists to + * serve kept working and the gap went unnoticed.) + * + * A component containing no '@' matches the name up to its '@', so both + * /soc/uart and /soc/uart@40011000 resolve. A component that does contain one + * is compared in full, so an explicit address still selects exactly the node + * asked for -- which matters when a tree has several of the same peripheral. + */ +static bool fdt_name_matches(const char *name, const char *comp, + uint32_t comp_len) +{ + uint32_t full = (uint32_t)strlen(name); + + if (memchr(comp, '@', comp_len) != NULL) { + return full == comp_len && strncmp(name, comp, comp_len) == 0; + } + + const char *at = memchr(name, '@', full); + uint32_t bare = at ? (uint32_t)(at - name) : full; + + return bare == comp_len && strncmp(name, comp, comp_len) == 0; +} + +int eos_fdt_validate(const void *fdt_blob, uint32_t avail) { if (!fdt_blob) return -1; - const fdt_header_t *hdr = (const fdt_header_t *)fdt_blob; - if (fdt32_to_cpu(hdr->magic) != FDT_MAGIC) return -2; - if (fdt32_to_cpu(hdr->version) < 16) return -3; + if (avail < sizeof(fdt_header_t)) return -6; + if (fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, magic)) != FDT_MAGIC) + return -2; + if (fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, version)) < 16) + return -3; + + /* Before anything else: the blob does not get to say how big it is. Every + * bound below is relative to totalsize, so without this the checks only + * prove the header is self-consistent — a 40-byte buffer claiming a 1 MiB + * totalsize with agreeing block offsets passes all of them, and the walk + * then runs a megabyte past the allocation. */ + if (fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, totalsize)) > avail) + return -6; + + /* magic and version alone say nothing about where the blob claims its + * blocks are. The struct and string offsets are read straight out of + * flash and then used as pointers, so a blob that clears the two checks + * above could still point them anywhere; eos_fdt_get_prop walked off the + * end of a 40-byte allocation and took a bus fault. Reject the blob here + * instead, because this is the gate every path goes through. */ + uint32_t totalsize = fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, totalsize)); + if (totalsize < sizeof(fdt_header_t)) return -6; + if (!fdt_block_in_bounds(fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, off_dt_struct)), + fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, size_dt_struct)), + totalsize)) + return -6; + if (!fdt_block_in_bounds(fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, off_dt_strings)), + fdt_hdr_u32(fdt_blob, offsetof(fdt_header_t, size_dt_strings)), + totalsize)) + return -6; return 0; } + int eos_fdt_load(uint32_t flash_addr, void *dest, uint32_t max_size) { if (!dest || max_size < sizeof(fdt_header_t)) return -1; @@ -43,11 +122,12 @@ int eos_fdt_load(uint32_t flash_addr, void *dest, uint32_t max_size) const void *src = (const void *)(uintptr_t)flash_addr; memcpy(dest, src, sizeof(fdt_header_t)); - int rc = eos_fdt_validate(dest); + /* Only the header has been copied so far, but max_size is what the caller + * really owns, so that is the bound the blob has to satisfy. */ + int rc = eos_fdt_validate(dest, max_size); if (rc != 0) return rc; - const fdt_header_t *hdr = (const fdt_header_t *)dest; - uint32_t total = fdt32_to_cpu(hdr->totalsize); + uint32_t total = fdt_hdr_u32(dest, offsetof(fdt_header_t, totalsize)); if (total > max_size) return -4; /* Copy full DTB */ @@ -55,15 +135,24 @@ int eos_fdt_load(uint32_t flash_addr, void *dest, uint32_t max_size) return 0; } -int eos_fdt_get_prop(const void *fdt, const char *node_path, - const char *prop_name, void *buf, uint32_t *buf_len) +int eos_fdt_get_prop(const void *fdt, uint32_t fdt_len, + const char *node_path, const char *prop_name, + void *buf, uint32_t *buf_len) { if (!fdt || !node_path || !prop_name || !buf || !buf_len) return -1; - const fdt_header_t *hdr = (const fdt_header_t *)fdt; - const uint8_t *dt_struct = (const uint8_t *)fdt + fdt32_to_cpu(hdr->off_dt_struct); - const char *dt_strings = (const char *)fdt + fdt32_to_cpu(hdr->off_dt_strings); - uint32_t struct_size = fdt32_to_cpu(hdr->size_dt_struct); + /* The blob is whatever was in flash. Every offset below comes out of its + * header, so none of them can be trusted until validate() has bounded + * them against both totalsize and the length the caller actually owns. */ + int vrc = eos_fdt_validate(fdt, fdt_len); + if (vrc != 0) return vrc; + + const uint8_t *dt_struct = (const uint8_t *)fdt + + fdt_hdr_u32(fdt, offsetof(fdt_header_t, off_dt_struct)); + const char *dt_strings = (const char *)fdt + + fdt_hdr_u32(fdt, offsetof(fdt_header_t, off_dt_strings)); + uint32_t struct_size = fdt_hdr_u32(fdt, offsetof(fdt_header_t, size_dt_struct)); + uint32_t strings_size = fdt_hdr_u32(fdt, offsetof(fdt_header_t, size_dt_strings)); /* Simple linear search through struct block */ uint32_t offset = 0; @@ -71,64 +160,141 @@ int eos_fdt_get_prop(const void *fdt, const char *node_path, int target_depth = -1; bool in_target = false; - /* Count slashes to determine target depth */ - int path_depth = 0; - for (const char *p = node_path; *p; p++) { - if (*p == '/') path_depth++; + /* Split the path into components. The old code counted slashes and + * compared only the last component, which was wrong twice over: the + * root node already occupies depth 1, so "/chosen" looked for depth 1 + * -- the root itself -- and never matched, leaving "/" the only path + * that resolved; and matching the last component alone meant + * "/soc/uart" would have accepted any node named "uart" at that depth, + * whatever its parent. */ + const char *comp[FDT_MAX_PATH_DEPTH]; + uint32_t comp_len[FDT_MAX_PATH_DEPTH]; + int ncomp = 0; + for (const char *p = node_path; *p; ) { + while (*p == '/') p++; + if (!*p) break; + const char *start = p; + while (*p && *p != '/') p++; + /* -8, not -1: -1 means a caller passed NULL, which is a programming + * error, while a path deeper than we track is input. A caller that + * cannot tell them apart cannot handle either correctly. */ + if (ncomp >= FDT_MAX_PATH_DEPTH) return -8; + comp[ncomp] = start; + comp_len[ncomp] = (uint32_t)(p - start); + ncomp++; } - if (path_depth == 0) path_depth = 1; + /* The root is depth 1, so a path of n components ends at depth n + 1. */ + const int target_path_depth = ncomp + 1; + int matched = 0; - while (offset < struct_size) { + /* offset < struct_size only guarantees one byte; every read below wants + * four or more, so each is checked for the width it actually takes. */ + while (offset + 4 <= struct_size) { uint32_t tag = fdt_read_u32(dt_struct + offset); offset += 4; switch (tag) { case FDT_BEGIN_NODE: { + /* strlen() here read until it happened to find a zero, which + * for a name running to the end of the block is past it. */ const char *name = (const char *)(dt_struct + offset); - uint32_t name_len = (uint32_t)strlen(name) + 1; + const void *nul = memchr(name, '\0', struct_size - offset); + if (!nul) return -6; + uint32_t name_len = (uint32_t)((const char *)nul - name) + 1; offset += (name_len + 3) & ~3U; + if (offset > struct_size) return -6; depth++; - /* Check if this node matches the target path */ - if (depth == path_depth) { - /* Simple check: compare last component */ - const char *last_slash = strrchr(node_path, '/'); - const char *target_name = last_slash ? last_slash + 1 : node_path; - if (strcmp(name, target_name) == 0 || target_name[0] == '\0') { - in_target = true; - target_depth = depth; + /* Advance along the requested path only while every ancestor + * has matched, so a node is found at its own path and not + * merely by its own name. */ + if (depth >= 2 && matched == depth - 2 && depth - 2 < ncomp) { + uint32_t want = comp_len[depth - 2]; + if (fdt_name_matches(name, comp[depth - 2], want)) { + matched = depth - 1; } } + if (!in_target && depth == target_path_depth && matched == ncomp) { + in_target = true; + target_depth = depth; + } break; } case FDT_END_NODE: if (in_target && depth == target_depth) in_target = false; + /* Leaving a node un-matches it for the branch we return to. */ + if (matched >= depth - 1 && depth >= 2) matched = depth - 2; + /* An unbalanced blob would drive depth negative and let a later + * BEGIN_NODE match path_depth at the wrong nesting level. */ + if (depth == 0) return -6; depth--; break; case FDT_PROP: { + if (offset + 8 > struct_size) return -6; uint32_t len = fdt_read_u32(dt_struct + offset); uint32_t nameoff = fdt_read_u32(dt_struct + offset + 4); offset += 8; + + /* nameoff indexes the strings block; unchecked it named any + * address, and strcmp then read from it. */ + if (nameoff >= strings_size) return -6; const char *pname = dt_strings + nameoff; + if (!memchr(pname, '\0', strings_size - nameoff)) return -6; + + /* The value has to be inside the struct block before it is read: + * copy_len was clamped to the caller's buffer, which bounded the + * write but not the read, so an oversized len leaked whatever + * followed the blob into buf. */ + if (len > struct_size - offset) return -6; - if (in_target && strcmp(pname, prop_name) == 0) { - uint32_t copy_len = len < *buf_len ? len : *buf_len; - memcpy(buf, dt_struct + offset, copy_len); - *buf_len = copy_len; + /* depth, not just in_target: in_target is cleared by the + * target's own END_NODE, so it stays true for the whole subtree + * and a property on a *child* was returned as the target's own. + * Real trees put properties before subnodes, so a property that + * is present on the target is still found first -- the bug bit + * when the target lacked it and "not found" became a silently + * wrong value from a nested node. depth is exactly target_depth + * for the target's own properties and target_depth + 1 or more + * inside any child. */ + if (in_target && depth == target_depth && + strcmp(pname, prop_name) == 0) { + /* Truncating and returning 0 told the caller it had the whole + * value. This path reads bootargs: a clipped string that + * reports success drops whatever sat at its end, and nothing + * distinguishes that from a short property. Report the full + * length so the caller can size a retry. */ + if (len > *buf_len) { + *buf_len = len; + return -7; + } + memcpy(buf, dt_struct + offset, len); + *buf_len = len; return 0; } + /* len is bounded above, so the pad cannot wrap. */ offset += (len + 3) & ~3U; + if (offset > struct_size) return -6; break; } case FDT_END: return -5; /* Property not found */ - default: + case FDT_NOP: + /* Legal padding; the spec allows it anywhere between tokens. */ break; + default: + /* Refuse, do not resynchronise. Skipping an unknown tag and + * treating whatever sits 4 bytes on as the next token meant a + * struct block of arbitrary bytes parsed to completion. Every + * read stayed bounded, so this was not memory-unsafe -- but a + * parser in the TCB that walks garbage to a clean "not found" + * is quietly accepting input it does not understand. */ + return -6; } } return -5; } + void eos_fdt_pass_to_kernel(uint32_t dtb_addr) { /* The DTB address is passed to the kernel via: diff --git a/include/eos_fdt_loader.h b/include/eos_fdt_loader.h index 30eec7b..d68bfa5 100644 --- a/include/eos_fdt_loader.h +++ b/include/eos_fdt_loader.h @@ -15,10 +15,16 @@ extern "C" { #endif +/* Longest node path this parser resolves, in components. A caller cannot + * act on return code -8 without knowing the limit, so it is declared here + * rather than privately in fdt_loader.c. */ +#define FDT_MAX_PATH_DEPTH 16 + #define FDT_MAGIC 0xD00DFEEDU #define FDT_BEGIN_NODE 0x00000001U #define FDT_END_NODE 0x00000002U #define FDT_PROP 0x00000003U +#define FDT_NOP 0x00000004U #define FDT_END 0x00000009U typedef struct { @@ -34,10 +40,57 @@ typedef struct { uint32_t size_dt_struct; } fdt_header_t; -int eos_fdt_validate(const void *fdt_blob); +/** + * Return codes. Every entry point below returns 0 on success and one of these + * on failure; they are distinct so a caller can tell a programming error from + * a malformed blob from a buffer that was too small. + * + * -1 a required argument was NULL + * -2 the blob does not carry the FDT magic + * -3 the blob declares an unsupported version (< 16) + * -4 the blob does not fit the destination buffer + * -5 no such node or property + * -6 the blob is malformed: an offset or length escapes it + * -7 the property is larger than the caller's buffer (see below) + * -8 the node path is deeper than FDT_MAX_PATH_DEPTH components + */ + +/** + * Validate a blob. + * + * @param avail bytes actually readable at @p fdt_blob. + * + * There is deliberately no length-free form. Every bound inside the header -- + * the struct and string block offsets and sizes -- is expressed relative to + * the header's own totalsize field, which is attacker-controlled; checking + * those against each other proves the blob is internally consistent and says + * nothing about how many bytes are really mapped. An earlier revision kept a + * one-argument wrapper that read totalsize and passed it as the bound, which + * is an out-of-bounds read on a short buffer before any check has run. A + * documented caller warrant is not a check, and an exported unsafe twin in a + * TCB header is a future boot-path caller reintroducing the bug with no + * compiler complaint. + */ +int eos_fdt_validate(const void *fdt_blob, uint32_t avail); + int eos_fdt_load(uint32_t flash_addr, void *dest, uint32_t max_size); -int eos_fdt_get_prop(const void *fdt, const char *node_path, - const char *prop_name, void *buf, uint32_t *buf_len); + +/** + * Read a property. + * + * @param fdt_len bytes actually readable at @p fdt. + * @param buf_len in: capacity of @p buf. out: bytes written on success, or + * the property's full length when -7 is returned, so the + * caller can size a retry. + * + * Returns -7 rather than truncating. A boot path reads bootargs through here, + * and a silently clipped value that reports success loses whatever sat at the + * end of the string. + */ +int eos_fdt_get_prop(const void *fdt, uint32_t fdt_len, + const char *node_path, const char *prop_name, + void *buf, uint32_t *buf_len); + void eos_fdt_pass_to_kernel(uint32_t dtb_addr); #ifdef __cplusplus diff --git a/include/eos_image.h b/include/eos_image.h index 62744d0..24cbb16 100644 --- a/include/eos_image.h +++ b/include/eos_image.h @@ -108,7 +108,8 @@ EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, tlv_hash) + /* Every remaining field, pinned. * - * Four of the fourteen fields were asserted. Transposing two adjacent + * Three of the thirteen field offsets were asserted (the fourth pre-existing + * assert is sizeof, which is not a field). Transposing two adjacent * same-width fields moves neither sizeof nor any of those four offsets, so it * compiled clean: with load_addr and entry_addr swapped, all four existing * asserts still passed and the bootloader would load an image at its entry @@ -132,15 +133,18 @@ EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, flags) == 24, "flags must stay at offset 24"); EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, sig_len) == 61, "sig_len must stay at offset 61"); -EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, reserved) == 62, - "reserved[] must stay at offset 62"); +/* tlv_len and tlv_hash are asserted above, where #93 introduced them; the + * 30 bytes they occupy are the ones this block used to pin as reserved[]. */ /* Field widths. An offset assert cannot see a field growing into padding that - * happens to keep every later offset -- reserved[] absorbs exactly that. */ + * happens to keep every later offset -- the 30 bytes at 62 absorb exactly + * that, which is why both halves of that span carry a width assert. */ EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->hash) == 32, "hash[] is 32 bytes on the wire"); -EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->reserved) == 30, - "reserved[] is 30 bytes on the wire"); +EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_len) == 2, + "tlv_len is 2 bytes on the wire"); +EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_hash) == 28, + "tlv_hash is 28 bytes on the wire"); EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->signature) == 64, "signature[] is 64 bytes on the wire"); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b394f24..9ffffd7 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -110,6 +110,12 @@ add_executable(eboot_test_ecc unit/test_ecc.c) target_link_libraries(eboot_test_ecc PRIVATE eboot_core) add_test(NAME test_ecc COMMAND eboot_test_ecc) +# --- test_fdt_loader: device tree parsing against malformed blobs --- +# core/fdt_loader.c was in no source list, so it had never been compiled. +add_executable(eboot_test_fdt_loader unit/test_fdt_loader.c) +target_link_libraries(eboot_test_fdt_loader PRIVATE eboot_core) +add_test(NAME test_fdt_loader COMMAND eboot_test_fdt_loader) + # --- Valgrind test targets --- find_program(VALGRIND valgrind) if(VALGRIND) diff --git a/tests/fuzz/CMakeLists.txt b/tests/fuzz/CMakeLists.txt index 85d6b71..633cf84 100644 --- a/tests/fuzz/CMakeLists.txt +++ b/tests/fuzz/CMakeLists.txt @@ -14,7 +14,7 @@ if(EBLDR_BUILD_FUZZ) return() endif() - set(FUZZ_FLAGS "-fsanitize=fuzzer,address -fno-omit-frame-pointer") + set(FUZZ_FLAGS "-fsanitize=fuzzer,address,undefined -fno-omit-frame-pointer") # --- fuzz_image_verify --- add_executable(fuzz_image_verify fuzz_image_verify.c) @@ -48,6 +48,14 @@ if(EBLDR_BUILD_FUZZ) LINK_FLAGS "${FUZZ_FLAGS}" ) + # --- fuzz_fdt --- + add_executable(fuzz_fdt fuzz_fdt.c) + target_link_libraries(fuzz_fdt PRIVATE eboot_core) + set_target_properties(fuzz_fdt PROPERTIES + COMPILE_FLAGS "${FUZZ_FLAGS}" + LINK_FLAGS "${FUZZ_FLAGS}" + ) + # --- fuzz_bootctl --- add_executable(fuzz_bootctl fuzz_bootctl.c) target_link_libraries(fuzz_bootctl PRIVATE eboot_core) @@ -56,5 +64,5 @@ if(EBLDR_BUILD_FUZZ) LINK_FLAGS "${FUZZ_FLAGS}" ) - message(STATUS " Fuzz targets: fuzz_image_verify, fuzz_recovery_protocol, fuzz_fw_update, fuzz_crypto, fuzz_bootctl") + message(STATUS " Fuzz targets: fuzz_image_verify, fuzz_recovery_protocol, fuzz_fw_update, fuzz_crypto, fuzz_bootctl, fuzz_fdt") endif() diff --git a/tests/fuzz/fuzz_fdt.c b/tests/fuzz/fuzz_fdt.c new file mode 100644 index 0000000..335fb59 --- /dev/null +++ b/tests/fuzz/fuzz_fdt.c @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project + +/** + * @file fuzz_fdt.c + * @brief libFuzzer harness for the flattened device tree parser + * + * The DTB is off-device input: it comes out of flash or from a prior boot + * stage, and every offset and length in its header is attacker-controlled. + * .ai/security.md names device tree among the parsers that "get fuzz + * coverage, not just unit tests"; core/fdt_loader.c had neither until + * recently, and the ten hand-written blobs in tests/unit/test_fdt_loader.c + * cover the shapes that were reasoned about rather than the ones nobody + * thought of. + * + * The size-carrying entry points are the ones driven here. Passing `size` + * is what makes the harness meaningful: the unsized forms take the blob's + * own totalsize as the bound, so a fuzzer that inflates that field would be + * telling the parser it may read past the buffer libFuzzer allocated, and + * every report would be the harness's fault rather than the parser's. + */ + +#include "eos_fdt_loader.h" + +#include +#include + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + if (size > UINT32_MAX) { + return 0; + } + + uint32_t len = (uint32_t)size; + + if (eos_fdt_validate(data, len) != 0) { + return 0; + } + + /* Only reached for a blob whose header survived validation, which is + * where the interesting walking bugs live. Both a path that exists in + * most trees and one that does not, so the FDT_END and not-found exits + * are exercised as well as the match. */ + unsigned char buf[256]; + uint32_t buf_len = sizeof buf; + (void)eos_fdt_get_prop(data, len, "/chosen", "bootargs", + buf, &buf_len); + + buf_len = sizeof buf; + (void)eos_fdt_get_prop(data, len, "/", "compatible", + buf, &buf_len); + + /* A one-byte buffer drives the -7 truncation path, which is the branch + * that reports a length back to the caller. */ + unsigned char tiny[1]; + buf_len = sizeof tiny; + (void)eos_fdt_get_prop(data, len, "/", "bootargs", tiny, &buf_len); + + return 0; +} diff --git a/tests/unit/test_ed25519.c b/tests/unit/test_ed25519.c index f78012d..9ea837c 100644 --- a/tests/unit/test_ed25519.c +++ b/tests/unit/test_ed25519.c @@ -30,6 +30,7 @@ static int tests_passed = 0; static void name(void); \ static void run_##name(void) { \ printf(" %-50s ", #name); \ + tests_run++; \ name(); \ tests_passed++; \ printf("[PASS]\n"); \ @@ -211,29 +212,105 @@ TEST(test_ed25519_identity_key_forgery_rejected) msg, sizeof(msg) - 1) != EOS_OK); } +/* The eight canonical low-order point encodings. + * + * Every order was computed rather than copied: decoding each y, recovering x, + * and repeatedly adding the point until it reached the identity gives + * 1, 2, 4, 4, 8, 8, 8, 8 for the entries below in order. An earlier revision + * of this array held only five of them -- it omitted y=0 with the sign bit + * set and both sign-flipped order-8 encodings -- while its comment claimed to + * hold "the eight". [L](-A) = -[L]A, so the guard rejects a sign variant + * whether or not it is listed; the reason to list them is that this is the + * regression record for a secure-boot bypass, and a claimed class has to be + * the class it claims. */ +static const uint8_t k_low_order[8][32] = { + /* order 1: the identity, y = 1 */ + {0x01}, + /* order 2: y = -1 */ + {0xEC,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x7F}, + /* order 4: y = 0, sign bit clear */ + {0x00}, + /* order 4: y = 0, sign bit set -- the encoding the earlier array missed */ + {0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80}, + /* order 8 */ + {0x26,0xE8,0x95,0x8F,0xC2,0xB2,0x27,0xB0,0x45,0xC3,0xF4,0x89,0xF2,0xEF,0x98,0xF0, + 0xD5,0xDF,0xAC,0x05,0xD3,0xC6,0x33,0x39,0xB1,0x38,0x02,0x88,0x6D,0x53,0xFC,0x05}, + /* order 8 */ + {0xC7,0x17,0x6A,0x70,0x3D,0x4D,0xD8,0x4F,0xBA,0x3C,0x0B,0x76,0x0D,0x10,0x67,0x0F, + 0x2A,0x20,0x53,0xFA,0x2C,0x39,0xCC,0xC6,0x4E,0xC7,0xFD,0x77,0x92,0xAC,0x03,0x7A}, + /* order 8: sign flip of the first order-8 entry -- also missing before */ + {0x26,0xE8,0x95,0x8F,0xC2,0xB2,0x27,0xB0,0x45,0xC3,0xF4,0x89,0xF2,0xEF,0x98,0xF0, + 0xD5,0xDF,0xAC,0x05,0xD3,0xC6,0x33,0x39,0xB1,0x38,0x02,0x88,0x6D,0x53,0xFC,0x85}, + /* order 8: sign flip of the second -- also missing before */ + {0xC7,0x17,0x6A,0x70,0x3D,0x4D,0xD8,0x4F,0xBA,0x3C,0x0B,0x76,0x0D,0x10,0x67,0x0F, + 0x2A,0x20,0x53,0xFA,0x2C,0x39,0xCC,0xC6,0x4E,0xC7,0xFD,0x77,0x92,0xAC,0x03,0xFA}, +}; + +/* Not low-order points, and refused earlier and by a different mechanism: + * unpackneg() rejects them on canonicality or because no x exists. Kept + * separate so the array above means what its name says -- an earlier revision + * spent one of its eight slots on D9FF..FF, which does not decode at all. */ +static const uint8_t k_non_canonical[3][32] = { + /* y = p: reduces to 0, decodes as an order-4 point but is not canonical */ + {0xED,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x7F}, + /* y = p + 1: reduces to the identity, likewise not canonical */ + {0xEE,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x7F}, + /* no x satisfies the curve equation for this y: unpackneg() fails */ + {0xD9,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF}, +}; + +/* A low-order key forges for roughly one message in n, where n is its order, + * so a single fixed message would let a real bypass pass this suite. */ +static const char *const messages[] = { + "untrusted firmware", "v1.0.0", "", "a", "boot", "eos", "1234", "payload", +}; + TEST(test_ed25519_low_order_keys_rejected) { /* zero_pubkey covers one encoding; Ed25519 has eight low-order points and * the family is what matters. A subgroup test alone is not enough either: * the identity has order 1, which divides L, so [L]identity = identity and - * it passes. Both checks are required. */ - static const uint8_t low_order[4][32] = { - {0}, - {1}, - {0x26,0xe8,0x95,0x8f,0xc2,0xb2,0x27,0xb0,0x45,0xc3,0xf4,0x89,0xf2,0xef,0x98,0xf0, - 0xd5,0xdf,0xac,0x05,0xd3,0xc6,0x33,0x39,0xb1,0x38,0x02,0x88,0x6d,0x53,0xfc,0x05}, - {0xc7,0x17,0x6a,0x70,0x3d,0x4d,0xd8,0x4f,0xba,0x3c,0x0b,0x76,0x0d,0x10,0x67,0x0f, - 0x2a,0x20,0x53,0xfa,0x2c,0x39,0xcc,0xc6,0x4e,0xc7,0xfd,0x77,0x92,0xac,0x03,0x7a}, - }; - const uint8_t msg[] = "untrusted firmware"; + * it passes. Both checks are required. + * + * The sweep is every low-order encoding as the key against every one as R, + * over eight messages, because a low-order key of order n forges for + * roughly one message in n -- a single fixed message would let a genuine + * bypass through this test. Measured against 13a7a02, the last commit + * before the subgroup check: 16 of the 64 (key, R) pairs were accepted by + * at least one message. Here: none. */ + for (size_t k = 0; k < sizeof(k_low_order) / sizeof(k_low_order[0]); k++) { + for (size_t r = 0; r < sizeof(k_low_order) / sizeof(k_low_order[0]); r++) { + for (size_t m = 0; m < sizeof(messages) / sizeof(messages[0]); m++) { + uint8_t sig[64]; + memset(sig, 0, sizeof(sig)); + memcpy(sig, k_low_order[r], 32); + ASSERT(eos_ed25519_verify(sig, k_low_order[k], + (const uint8_t *)messages[m], + strlen(messages[m])) != EOS_OK); + } + } + } +} - for (int k = 0; k < 4; k++) { - for (int r = 0; r < 4; r++) { +TEST(test_ed25519_non_canonical_encodings_rejected) +{ + /* These are refused before the subgroup check ever runs -- unpackneg() + * rejects them on canonicality, or because no x satisfies the curve + * equation. Pinned separately so that nobody deletes that path on the + * grounds that the subgroup test now covers it. It does not. */ + for (size_t k = 0; k < sizeof(k_non_canonical) / sizeof(k_non_canonical[0]); k++) { + for (size_t m = 0; m < sizeof(messages) / sizeof(messages[0]); m++) { uint8_t sig[64]; memset(sig, 0, sizeof(sig)); - memcpy(sig, low_order[r], 32); - ASSERT(eos_ed25519_verify(sig, low_order[k], - msg, sizeof(msg) - 1) != EOS_OK); + memcpy(sig, k_non_canonical[k], 32); + ASSERT(eos_ed25519_verify(sig, k_non_canonical[k], + (const uint8_t *)messages[m], + strlen(messages[m])) != EOS_OK); } } } @@ -260,21 +337,6 @@ TEST(test_ed25519_zero_signature_rejected) ASSERT(eos_ed25519_verify(sig, pk, msg, 1) != EOS_OK); } -TEST(test_ed25519_identity_key_forgery_rejected) -{ - /* The identity point has compressed encoding 01 00...00. With both the - * public key and R set to the identity and S set to zero, the verification - * equation is true for every message unless low-order keys are rejected. */ - uint8_t identity_pub[32] = {1}; - uint8_t identity_sig[64] = {1}; - const uint8_t msg[] = "untrusted firmware"; - - ASSERT(eos_ed25519_verify(identity_sig, identity_pub, - msg, sizeof(msg) - 1) != EOS_OK); -} - -/* ---- SHA-512, the hash Ed25519 is defined over (FIPS 180-4) ---- */ - TEST(test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery) { /* The subgroup check guards the public key, not R, and that is @@ -367,13 +429,13 @@ int main(void) run_test_ed25519_null_args(); run_test_ed25519_identity_key_forgery_rejected(); run_test_ed25519_low_order_keys_rejected(); + run_test_ed25519_non_canonical_encodings_rejected(); + run_test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery(); run_test_ed25519_zero_pubkey_rejected(); run_test_ed25519_zero_signature_rejected(); - run_test_ed25519_identity_key_forgery_rejected(); run_test_sha512_known_answers(); run_test_sha512_streaming_matches_one_shot(); - tests_run = 11; printf("\n%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; } diff --git a/tests/unit/test_fdt_loader.c b/tests/unit/test_fdt_loader.c new file mode 100644 index 0000000..3a68ae5 --- /dev/null +++ b/tests/unit/test_fdt_loader.c @@ -0,0 +1,632 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project +// ISO/IEC 25000 | ISO/IEC/IEEE 15288:2023 + +/** + * @file test_fdt_loader.c + * @brief Unit tests for the FDT loader against malformed device trees + * + * core/fdt_loader.c parses a blob that comes out of flash, so every offset + * and length in its header is attacker-controlled. The file was in no + * CMakeLists, so none of this was ever compiled, let alone run. + * + * Each negative case below is a blob that eos_fdt_validate() used to accept + * -- it checked only magic and version -- and that then drove + * eos_fdt_get_prop() off the end of the allocation. The positive case is + * here to keep the bounds checks from simply rejecting everything. + */ + +#include "eos_fdt_loader.h" +#include +#include +#include +#include + +static int tests_passed = 0; + +#define ASSERT(condition) \ + do { \ + if (!(condition)) { \ + fprintf(stderr, "[FAIL] %s:%d: %s\n", \ + __FILE__, __LINE__, #condition); \ + exit(1); \ + } \ + } while (0) + +#define RUN(test) \ + do { \ + test(); \ + tests_passed++; \ + printf("[PASS] %s\n", #test); \ + } while (0) + +/* ---- DTB construction helpers ---------------------------------------- */ + +#define BLOB_MAX 512 + +typedef struct { + unsigned char bytes[BLOB_MAX]; + uint32_t len; +} blob_t; + +static void put_be32(unsigned char *p, uint32_t v) +{ + p[0] = (unsigned char)(v >> 24); p[1] = (unsigned char)(v >> 16); + p[2] = (unsigned char)(v >> 8); p[3] = (unsigned char)v; +} + +static void append_be32(blob_t *b, uint32_t v) +{ + ASSERT(b->len + 4 <= BLOB_MAX); + put_be32(b->bytes + b->len, v); + b->len += 4; +} + +/* Append a NUL-terminated string padded to a 4-byte boundary. */ +static void append_padded(blob_t *b, const char *s) +{ + uint32_t n = (uint32_t)strlen(s) + 1; + ASSERT(b->len + ((n + 3) & ~3U) <= BLOB_MAX); + memcpy(b->bytes + b->len, s, n); + b->len += n; + while (b->len & 3U) b->bytes[b->len++] = 0; +} + +/* A minimal well-formed tree: / { bootargs = "ro"; chosen { }; } + * + * Layout is header, then the struct block, then the strings block, so the + * offsets the header carries are the real ones. + * + * The property sits on the root and the tests query "/". Node paths below + * the root do not resolve: _get_prop derives the depth to match at by + * counting slashes, so "/" looks for depth 1 -- which is the root + * itself, the node "chosen" being at depth 2. That is a separate defect in + * the same never-compiled file and is not what these tests are about, so + * they stay on the one path form the matcher handles. + */ +static void build_valid(blob_t *b, const char *value) +{ + memset(b, 0, sizeof(*b)); + b->len = sizeof(fdt_header_t); + uint32_t off_struct = b->len; + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, ""); + + uint32_t vlen = (uint32_t)strlen(value) + 1; + append_be32(b, FDT_PROP); + append_be32(b, vlen); + append_be32(b, 0); /* nameoff: "bootargs" at strings[0] */ + append_padded(b, value); + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "chosen"); + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END); + uint32_t size_struct = b->len - off_struct; + + uint32_t off_strings = b->len; + append_padded(b, "bootargs"); + uint32_t size_strings = b->len - off_strings; + + fdt_header_t *h = (fdt_header_t *)b->bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->last_comp_version, 16); + put_be32((unsigned char *)&h->totalsize, b->len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, size_strings); +} + +/* / { soc { uart { reg = ; }; }; decoy { }; } + * + * "uart" sits at depth 3 under "soc", and a second node named "uart" is + * placed under "decoy" so that matching by last component alone would find + * the wrong one. */ +static void build_nested(blob_t *b, const char *value, const char *decoy) +{ + memset(b, 0, sizeof(*b)); + b->len = sizeof(fdt_header_t); + uint32_t off_struct = b->len; + uint32_t vlen = (uint32_t)strlen(value) + 1; + uint32_t dlen = (uint32_t)strlen(decoy) + 1; + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, ""); + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "decoy"); + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "uart"); + append_be32(b, FDT_PROP); append_be32(b, dlen); append_be32(b, 0); + append_padded(b, decoy); + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END_NODE); + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "soc"); + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "uart"); + append_be32(b, FDT_PROP); append_be32(b, vlen); append_be32(b, 0); + append_padded(b, value); + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END_NODE); + + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END); + uint32_t size_struct = b->len - off_struct; + + uint32_t off_strings = b->len; + append_padded(b, "reg"); + uint32_t size_strings = b->len - off_strings; + + fdt_header_t *h = (fdt_header_t *)b->bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->last_comp_version, 16); + put_be32((unsigned char *)&h->totalsize, b->len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, size_strings); +} + +/* Like build_nested, but with the unit addresses a real device tree carries. + * Exact-equality matching resolved nothing here, which is the case the PR's + * own headline example describes and could not do. */ +static void build_addressed(blob_t *b, const char *v0, const char *v1) +{ + memset(b, 0, sizeof(*b)); + b->len = sizeof(fdt_header_t); + uint32_t off_struct = b->len; + uint32_t l0 = (uint32_t)strlen(v0) + 1; + uint32_t l1 = (uint32_t)strlen(v1) + 1; + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, ""); + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "soc"); + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "uart@40011000"); + append_be32(b, FDT_PROP); append_be32(b, l0); append_be32(b, 0); + append_padded(b, v0); + append_be32(b, FDT_END_NODE); + + append_be32(b, FDT_BEGIN_NODE); append_padded(b, "uart@40004400"); + append_be32(b, FDT_PROP); append_be32(b, l1); append_be32(b, 0); + append_padded(b, v1); + append_be32(b, FDT_END_NODE); + + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END_NODE); + append_be32(b, FDT_END); + uint32_t size_struct = b->len - off_struct; + + uint32_t off_strings = b->len; + append_padded(b, "reg"); + uint32_t size_strings = b->len - off_strings; + + fdt_header_t *h = (fdt_header_t *)b->bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->last_comp_version, 16); + put_be32((unsigned char *)&h->totalsize, b->len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, size_strings); +} + +static void set_field(blob_t *b, size_t field_offset, uint32_t v) +{ + put_be32(b->bytes + field_offset, v); +} + +#define FIELD(name) offsetof(fdt_header_t, name) + +/* Run get_prop on a heap copy sized exactly to the blob, so an + * out-of-bounds read lands in a sanitizer's redzone rather than in + * whatever the test's own stack happens to hold. */ +static int get_prop_exact(const blob_t *b, const char *node, const char *prop, + void *out, uint32_t *out_len) +{ + unsigned char *heap = malloc(b->len); + ASSERT(heap != NULL); + memcpy(heap, b->bytes, b->len); + int rc = eos_fdt_get_prop(heap, b->len, node, prop, out, out_len); + free(heap); + return rc; +} + +/* ---- Tests ------------------------------------------------------------ */ + +static void test_valid_tree_round_trips(void) +{ + blob_t b; build_valid(&b, "ro"); + ASSERT(eos_fdt_validate(b.bytes, b.len) == 0); + + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) == 0); + ASSERT(len == 3); + ASSERT(strcmp(out, "ro") == 0); +} + +static void test_missing_property_is_reported(void) +{ + blob_t b; build_valid(&b, "ro"); + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "nonesuch", out, &len) != 0); +} + +static void test_struct_offset_past_the_blob_is_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + set_field(&b, FIELD(off_dt_struct), 0x100000); + + ASSERT(eos_fdt_validate(b.bytes, b.len) != 0); + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_strings_offset_past_the_blob_is_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + set_field(&b, FIELD(off_dt_strings), 0x100000); + + ASSERT(eos_fdt_validate(b.bytes, b.len) != 0); + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_struct_size_running_past_the_blob_is_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + set_field(&b, FIELD(size_dt_struct), 0xFFFFFF00U); + + ASSERT(eos_fdt_validate(b.bytes, b.len) != 0); + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_totalsize_below_the_header_is_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + set_field(&b, FIELD(totalsize), 8); + ASSERT(eos_fdt_validate(b.bytes, b.len) != 0); +} + +static void test_property_length_past_the_struct_block_is_rejected(void) +{ + /* The write was clamped to the caller's buffer, so this never + * overflowed buf -- it read past the blob and copied what followed it + * out to the caller. */ + blob_t b; build_valid(&b, "ro"); + + /* Find the FDT_PROP tag and enlarge its length field. */ + for (uint32_t i = sizeof(fdt_header_t); i + 8 <= b.len; i += 4) { + if (b.bytes[i] == 0 && b.bytes[i+1] == 0 && + b.bytes[i+2] == 0 && b.bytes[i+3] == FDT_PROP) { + set_field(&b, i + 4, 0xFFFF0000U); + break; + } + } + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_property_nameoff_past_the_strings_block_is_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + for (uint32_t i = sizeof(fdt_header_t); i + 12 <= b.len; i += 4) { + if (b.bytes[i] == 0 && b.bytes[i+1] == 0 && + b.bytes[i+2] == 0 && b.bytes[i+3] == FDT_PROP) { + set_field(&b, i + 8, 0xFFFF0000U); /* nameoff */ + break; + } + } + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_unterminated_node_name_is_rejected(void) +{ + /* A node name that runs to the end of the struct block with no NUL: + * strlen() used to read straight past it. */ + blob_t b; memset(&b, 0, sizeof b); + b.len = sizeof(fdt_header_t); + uint32_t off_struct = b.len; + append_be32(&b, FDT_BEGIN_NODE); + for (int i = 0; i < 8; i++) b.bytes[b.len++] = 'A'; + uint32_t size_struct = b.len - off_struct; + uint32_t off_strings = b.len; + b.bytes[b.len++] = 0; b.len += 3; + + fdt_header_t *h = (fdt_header_t *)b.bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->totalsize, b.len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, 4); + + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +static void test_null_arguments_are_rejected(void) +{ + blob_t b; build_valid(&b, "ro"); + char out[16]; uint32_t len = sizeof out; + ASSERT(eos_fdt_validate(NULL, 0) != 0); + ASSERT(eos_fdt_get_prop(NULL, b.len, "/", "bootargs", out, &len) != 0); + ASSERT(eos_fdt_get_prop(b.bytes, b.len, NULL, "bootargs", out, &len) != 0); + ASSERT(eos_fdt_get_prop(b.bytes, b.len, "/", NULL, out, &len) != 0); + ASSERT(eos_fdt_get_prop(b.bytes, b.len, "/", "bootargs", NULL, &len) != 0); + ASSERT(eos_fdt_get_prop(b.bytes, b.len, "/", "bootargs", out, NULL) != 0); +} + +/* The mirror image of the offset tests above. Those inflate an offset and + * leave totalsize honest; this leaves every offset internally consistent and + * inflates totalsize itself. Nothing in the header contradicts anything else, + * so a check that bounds the blob against its own totalsize has nothing to + * catch -- the only thing that knows better is the caller's allocation. */ +static void test_an_inflated_totalsize_is_rejected_when_the_length_is_known(void) +{ + blob_t b; build_valid(&b, "ro"); + uint32_t honest = b.len; + set_field(&b, FIELD(totalsize), 0x100000); + b.len = honest; /* the allocation stays what it really was */ + + char out[16]; uint32_t len = sizeof out; + + /* Told the truth about the buffer, the parser refuses it. */ + ASSERT(eos_fdt_validate(b.bytes, honest) != 0); + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) != 0); +} + +/* A property that does not fit used to be copied short and reported as a + * success, so a clipped bootargs was indistinguishable from a complete one. */ +static void test_a_property_too_large_for_the_buffer_is_not_truncated(void) +{ + blob_t b; build_valid(&b, "root=/dev/mmcblk0p2 rw quiet"); + + char out[8]; + uint32_t len = sizeof out; + memset(out, 0xAA, sizeof out); + + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) == -7); + /* and it reports what the caller would need to allocate */ + ASSERT(len == sizeof "root=/dev/mmcblk0p2 rw quiet"); + /* nothing was written into the short buffer */ + ASSERT(out[0] == (char)0xAA); +} + +static void test_a_property_that_exactly_fits_still_succeeds(void) +{ + blob_t b; build_valid(&b, "ro"); + char out[3]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) == 0); + ASSERT(len == 3); + ASSERT(strcmp(out, "ro") == 0); +} + +/* An unrecognised tag used to be skipped -- the walk resynchronised 4 bytes + * on and a struct block of arbitrary bytes parsed to a clean "not found". + * Bounded, but a TCB parser that walks garbage to completion is accepting + * input it does not understand. Now only FDT_NOP passes. */ +static void test_a_garbage_tag_is_refused_not_skipped(void) +{ + blob_t b; + memset(&b, 0, sizeof(b)); + b.len = sizeof(fdt_header_t); + uint32_t off_struct = b.len; + append_be32(&b, FDT_BEGIN_NODE); append_padded(&b, ""); + append_be32(&b, 0xDEADBEEFU); /* not a token */ + append_be32(&b, FDT_END_NODE); + append_be32(&b, FDT_END); + uint32_t size_struct = b.len - off_struct; + uint32_t off_strings = b.len; + append_padded(&b, "bootargs"); + uint32_t size_strings = b.len - off_strings; + fdt_header_t *h = (fdt_header_t *)b.bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->last_comp_version, 16); + put_be32((unsigned char *)&h->totalsize, b.len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, size_strings); + + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) == -6); +} + +/* And the one legal unknown, FDT_NOP, is padding the spec allows anywhere + * between tokens -- refusing it would reject real device trees. */ +static void test_nop_padding_does_not_break_resolution(void) +{ + blob_t b; + memset(&b, 0, sizeof(b)); + b.len = sizeof(fdt_header_t); + uint32_t off_struct = b.len; + append_be32(&b, FDT_NOP); + append_be32(&b, FDT_BEGIN_NODE); append_padded(&b, ""); + append_be32(&b, FDT_NOP); + append_be32(&b, FDT_PROP); append_be32(&b, 3); append_be32(&b, 0); + append_padded(&b, "ro"); + append_be32(&b, FDT_NOP); + append_be32(&b, FDT_END_NODE); + append_be32(&b, FDT_END); + uint32_t size_struct = b.len - off_struct; + uint32_t off_strings = b.len; + append_padded(&b, "bootargs"); + uint32_t size_strings = b.len - off_strings; + fdt_header_t *h = (fdt_header_t *)b.bytes; + put_be32((unsigned char *)&h->magic, FDT_MAGIC); + put_be32((unsigned char *)&h->version, 17); + put_be32((unsigned char *)&h->last_comp_version, 16); + put_be32((unsigned char *)&h->totalsize, b.len); + put_be32((unsigned char *)&h->off_dt_struct, off_struct); + put_be32((unsigned char *)&h->size_dt_struct, size_struct); + put_be32((unsigned char *)&h->off_dt_strings, off_strings); + put_be32((unsigned char *)&h->size_dt_strings, size_strings); + + char out[16]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/", "bootargs", out, &len) == 0); + ASSERT(strcmp(out, "ro") == 0); +} + +/* The blob arrives wherever it arrives -- fuzz input, a buffer inside a + * larger message -- and nothing promises 4-byte alignment. Header fields go + * through the same memcpy rule as the struct block now; this pins it by + * parsing from an odd offset, which UBSan's alignment check turns into a + * hard failure if a direct member load ever comes back. */ +static void test_an_unaligned_blob_parses(void) +{ + blob_t b; build_valid(&b, "ro"); + + unsigned char *heap = malloc(b.len + 1); + ASSERT(heap != NULL); + memcpy(heap + 1, b.bytes, b.len); + + char out[16]; uint32_t len = sizeof out; + ASSERT(eos_fdt_validate(heap + 1, b.len) == 0); + ASSERT(eos_fdt_get_prop(heap + 1, b.len, "/", "bootargs", out, &len) == 0); + ASSERT(strcmp(out, "ro") == 0); + free(heap); +} + +static void test_a_node_below_the_root_resolves(void) +{ + /* "/chosen" used to look for depth 1 -- the root -- so no path below + * the root ever resolved and only "/" worked. + * + * This assertion is also what proves the parent is matched, and that is + * load-bearing rather than incidental: build_nested emits "decoy" before + * "soc", so a matcher comparing only the last component would meet + * /decoy/uart first and return "decoy-uart". Getting "soc-uart" here + * means the ancestors were checked. */ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart", "reg", out, &len) == 0); + ASSERT(strcmp(out, "soc-uart") == 0); +} + +static void test_each_uart_returns_its_own_value(void) +{ + /* Matching the last component alone would return decoy/uart's value + * for /soc/uart, whichever the walk reached first. */ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/decoy/uart", "reg", out, &len) == 0); + ASSERT(strcmp(out, "decoy-uart") == 0); +} + +static void test_a_path_that_is_not_in_the_tree_is_not_found(void) +{ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/spi", "reg", out, &len) != 0); + len = sizeof out; + ASSERT(get_prop_exact(&b, "/nosuch/uart", "reg", out, &len) != 0); +} + +static void test_a_deeper_path_than_the_tree_is_not_found(void) +{ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart/child", "reg", out, &len) != 0); +} + +/* The property arm used to test in_target alone, and in_target is only + * cleared by the target's own END_NODE -- so it stayed true through the whole + * subtree and a child's property came back as the parent's. /soc has no "reg" + * of its own; the correct answer is "not found", and what came back was + * /soc/uart's value. That is the same class of mistake this PR is named for: + * reading the wrong node's registers. */ +static void test_a_property_on_a_child_is_not_returned_as_the_parents(void) +{ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc", "reg", out, &len) != 0); + + /* and the child itself still resolves, so the guard is not just + * refusing everything below the root */ + len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart", "reg", out, &len) == 0); + ASSERT(strcmp(out, "soc-uart") == 0); +} + +/* A path deeper than FDT_MAX_PATH_DEPTH is input, not a programming error, + * and used to share -1 with "a caller passed NULL". */ +static void test_an_overlong_path_is_distinguishable_from_a_null_argument(void) +{ + blob_t b; build_nested(&b, "soc-uart", "decoy-uart"); + char out[24]; uint32_t len = sizeof out; + + char deep[128] = ""; + for (int i = 0; i < 20; i++) strcat(deep, "/a"); + + ASSERT(get_prop_exact(&b, deep, "reg", out, &len) == -8); + len = sizeof out; + ASSERT(eos_fdt_get_prop(NULL, b.len, "/soc", "reg", out, &len) == -1); +} + +/* The headline case: a path written without a unit address, against a tree + * that has one. Exact string equality never resolved this, so /soc/uart did + * not work on any real DTB -- only on trees built without addresses, which is + * what every other fixture in this file constructs. */ +static void test_a_path_without_a_unit_address_resolves_a_node_with_one(void) +{ + blob_t b; build_addressed(&b, "uart0", "uart1"); + char out[24]; uint32_t len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart", "reg", out, &len) == 0); + /* The first match wins, as it does for any other duplicate name. */ + ASSERT(strcmp(out, "uart0") == 0); +} + +/* And an explicit address still selects exactly the node asked for, which is + * what makes the loose match safe on a tree with several of a peripheral. */ +static void test_an_explicit_unit_address_selects_that_node(void) +{ + blob_t b; build_addressed(&b, "uart0", "uart1"); + char out[24]; uint32_t len = sizeof out; + + ASSERT(get_prop_exact(&b, "/soc/uart@40004400", "reg", out, &len) == 0); + ASSERT(strcmp(out, "uart1") == 0); + + len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart@40011000", "reg", out, &len) == 0); + ASSERT(strcmp(out, "uart0") == 0); + + /* An address that is not in the tree must not fall back to a loose match. */ + len = sizeof out; + ASSERT(get_prop_exact(&b, "/soc/uart@deadbeef", "reg", out, &len) != 0); +} + +int main(void) +{ + printf("=== eBootloader FDT Loader Tests ===\n"); + RUN(test_valid_tree_round_trips); + RUN(test_missing_property_is_reported); + RUN(test_struct_offset_past_the_blob_is_rejected); + RUN(test_strings_offset_past_the_blob_is_rejected); + RUN(test_struct_size_running_past_the_blob_is_rejected); + RUN(test_totalsize_below_the_header_is_rejected); + RUN(test_property_length_past_the_struct_block_is_rejected); + RUN(test_property_nameoff_past_the_strings_block_is_rejected); + RUN(test_unterminated_node_name_is_rejected); + RUN(test_null_arguments_are_rejected); + RUN(test_an_inflated_totalsize_is_rejected_when_the_length_is_known); + RUN(test_a_property_too_large_for_the_buffer_is_not_truncated); + RUN(test_a_property_that_exactly_fits_still_succeeds); + RUN(test_a_garbage_tag_is_refused_not_skipped); + RUN(test_nop_padding_does_not_break_resolution); + RUN(test_an_unaligned_blob_parses); + RUN(test_a_node_below_the_root_resolves); + RUN(test_each_uart_returns_its_own_value); + RUN(test_a_path_that_is_not_in_the_tree_is_not_found); + RUN(test_a_deeper_path_than_the_tree_is_not_found); + RUN(test_a_property_on_a_child_is_not_returned_as_the_parents); + RUN(test_an_overlong_path_is_distinguishable_from_a_null_argument); + RUN(test_a_path_without_a_unit_address_resolves_a_node_with_one); + RUN(test_an_explicit_unit_address_selects_that_node); + printf("\n%d tests passed\n", tests_passed); + return 0; +}