From f704d87fd445e4d61db00bc302e78660fc99f02f Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Thu, 3 Sep 2026 15:06:59 +0530 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20repair=20master=20=E2=80=94=20the=20?= =?UTF-8?q?ABI=20asserts=20and=20the=20Ed25519=20verifier=20both=20merged?= =?UTF-8?q?=20broken?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit master (22d8f8b) does not compile. Two independent double-merges, both the same shape: two PRs fixing adjacent things landed on stale bases, each was green on its own branch, and the result was never rebuilt. 1. include/eos_image.h — #93 replaced reserved[30] with tlv_len (2) + tlv_hash[28], preserving every offset. #87 merged afterwards carrying asserts written against the older struct: error: no member named 'reserved' in 'eos_image_header_t' (x2) #93 already asserts tlv_len at 62 and tlv_hash at 64, so the offset assert was a duplicate; the width assert had no replacement and is restored as two asserts covering both halves of the same 30-byte span. No offset moves and the wire format is unchanged. 2. core/ed25519_verify.c — #86 and #57 both landed a subgroup guard, so the file carried two byte-identical point_is_identity() definitions: error: redefinition of 'point_is_identity' Only #57's public_key_is_valid_subgroup() is wired to the call site, so #86's key_has_prime_order() was dead. Kept the live function, folded #86's fuller rationale onto it, deleted the duplicate. 3. tests/unit/test_ed25519.c — collateral from the same merge. Two copies of test_ed25519_identity_key_forgery_rejected, main() calling it twice and two tests not at all, and test_ed25519_low_order_R_with_a_valid_key_is_not_a_forgery referencing k_low_order[] and messages[] that the merge had dropped. While restoring the corpus, corrected it (review finding on #86): the array claimed to hold "the eight low-order point encodings" and held five. Every order here was computed rather than copied — decode y, recover x, add the point to itself until it reaches the identity — giving 1, 2, 4, 4, 8, 8, 8, 8. Missing before: y=0 with the sign bit set, and both sign-flipped order-8 encodings. D9FF..FF was in the array and is not a low-order point at all — no x satisfies the curve equation for that y — so it moves to a separate k_non_canonical[], with EDFF..FF7F (y=p) and EEFF..FF7F (y=p+1). tests_run was assigned a literal (11) in main() and never incremented, which is how the duplicate call and the two unregistered tests went unnoticed. The TEST macro now increments it, so the total cannot drift. Verified: cmake -DEBLDR_BUILD_TESTS=ON on master FAILS to build, 3 errors same with this commit builds clean ctest 21/21 PASS ctest -DEBLDR_SANITIZE=ON (ASan+UBSan) 21/21 PASS pytest tests/ 24 passed, 1 skipped test_ed25519 14/14 PASS (was 11 claimed, 12 run) discrimination, with `public_key_is_valid_subgroup` disabled: test_ed25519_low_order_keys_rejected FAILS, as it must test_ed25519_non_canonical_... still PASSES — those are refused by unpackneg() on canonicality, a different mechanism, which is the reason they are held in a separate array rather than counted among the eight. --- core/ed25519_verify.c | 56 ++++------------- include/eos_image.h | 16 +++-- tests/unit/test_ed25519.c | 126 ++++++++++++++++++++++++++++---------- 3 files changed, 116 insertions(+), 82 deletions(-) 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/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/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; } From 0ca7698f37efe73a1fd20ff42cc3a29d262ced32 Mon Sep 17 00:00:00 2001 From: kartikey1306 Date: Thu, 3 Sep 2026 23:13:23 +0530 Subject: [PATCH 2/2] fix(fuzz): point the harnesses at functions that exist, and build them in CI Three of the five fuzz harnesses named functions this tree has never had, and a fourth declared the wrong signature for one it does. They compiled -- a declaration costs nothing -- and never linked, because EBLDR_BUILD_FUZZ defaults OFF and no CI job set it. This is the third instance of the pattern: eos#50 found fuzz_devicetree declaring an eos_dtb_parse() that did not exist, and it was found the same way, by making something build it. Found by the fuzz-build job below, which was written for eBoot#84 and turned red immediately on harnesses that PR had not touched: undefined reference to `eos_fw_update_init' undefined reference to `eos_fw_update_process_chunk' undefined reference to `eos_recovery_parse_packet' | harness | was | now | |---|---|---| | fuzz_fw_update | `eos_fw_update_init/_process_chunk/_finalize(void)` -- none exist | real begin -> write -> finalize over a simulated flash, chunk widths from the input so a header can straddle two writes | | fuzz_recovery_protocol | `eos_recovery_parse_packet()` -- does not exist | `eos_recovery_write_in_range()` with all four arguments fuzzed, and a trap if it accepts a write that does not fit | | fuzz_bootctl | `eos_bootctl_parse()` -- does not exist | `eos_bootctl_load()` over fuzzer bytes staged as flash, then the transitions a boot makes on what it accepted | | fuzz_image_verify | `(const void *, size_t)` for a function that takes `(uint32_t, eos_image_header_t *)` | includes the header so the compiler checks the call; parses from an address, as the real caller does | | fuzz_crypto | correct | unchanged | The image_verify one is worth naming separately: C has no overloading, so that symbol matched and it would have linked and run, writing a parsed header through whatever `size` happened to be. A wrong declaration is worse than a missing one, because it is the case that does not fail loudly. eos_recovery_enter() is deliberately not driven -- a command loop over a real UART that does not return on success is not a shape libFuzzer can use. The range check is the part of recovery that takes untrusted numbers and answers a safety question. The CI job builds every harness and runs each briefly over its own generated inputs. Not a campaign -- enough to catch a harness that no longer compiles, no longer links, or crashes at once, which is the failure this repo has actually had, three times. It needs EBLDR_BUILD_TESTS alongside EBLDR_BUILD_FUZZ: tests/fuzz/ is added from tests/CMakeLists.txt, so the fuzz flag alone configures cleanly and builds nothing, and the job would pass having compiled no harness. Found that before writing the job rather than after. Verified: ctest 22/22 PASS pytest tests/ 38 passed all five harnesses link against eboot_core, checked with a stub main (this host has no libFuzzer runtime -- libclang_rt.fuzzer_osx.a is absent from the Xcode toolchain -- so the libFuzzer link itself is CI's to show) each harness under -fsanitize=address,undefined over 300 random inputs: fuzz_bootctl / fuzz_crypto / fuzz_fw_update / fuzz_image_verify / fuzz_recovery_protocol -- 300 inputs, no report, all five Refs #84, #50 --- .github/workflows/ci.yml | 39 ++++++++++++ tests/fuzz/fuzz_bootctl.c | 77 +++++++++++++++++++++--- tests/fuzz/fuzz_fw_update.c | 93 +++++++++++++++++++++++------ tests/fuzz/fuzz_image_verify.c | 38 +++++++++++- tests/fuzz/fuzz_recovery_protocol.c | 56 ++++++++++++++--- 5 files changed, 263 insertions(+), 40 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1cfbad9..5151553 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -124,6 +124,45 @@ jobs: retention-days: 90 # ── Static Analysis ─────────────────────────────────────────────────────── + # tests/fuzz/ was compiled by nothing. EBLDR_BUILD_FUZZ defaults OFF and no + # job set it, so five harnesses sat in the tree for as long as they have + # existed without ever being linked -- and three of them named functions + # that do not exist. A harness that is never built cannot fail to build. + # + # This does not fuzz. It builds every harness and runs each briefly over its + # own generated inputs, which is what catches a harness that no longer + # compiles, no longer links, or crashes at once. A real campaign belongs in + # a scheduled workflow. + # + # EBLDR_BUILD_TESTS is needed alongside EBLDR_BUILD_FUZZ: tests/fuzz/ is + # added from tests/CMakeLists.txt, so the fuzz flag alone configures cleanly + # and builds nothing -- this job would pass having compiled no harness. + fuzz-build: + name: Fuzz Harness Build + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + - name: Build fuzz targets + run: | + set -euo pipefail + cmake -B build/fuzz -DEBLDR_BUILD_TESTS=ON -DEBLDR_BUILD_FUZZ=ON \ + -DCMAKE_C_COMPILER=clang -DCMAKE_BUILD_TYPE=Debug + cmake --build build/fuzz -j"$(nproc)" + - name: Smoke-run each harness + run: | + set -euo pipefail + shopt -s nullglob + harnesses=(build/fuzz/tests/fuzz/fuzz_*) + if [ ${#harnesses[@]} -eq 0 ]; then + echo "::error::EBLDR_BUILD_FUZZ=ON produced no harnesses" + exit 1 + fi + echo "built ${#harnesses[@]} harnesses" + for f in "${harnesses[@]}"; do + echo "=== $(basename "$f") ===" + "$f" -max_total_time=5 -print_final_stats=1 + done + static-analysis: name: Static Analysis (cppcheck + clang-tidy) runs-on: ubuntu-22.04 diff --git a/tests/fuzz/fuzz_bootctl.c b/tests/fuzz/fuzz_bootctl.c index c52ab54..76841cb 100644 --- a/tests/fuzz/fuzz_bootctl.c +++ b/tests/fuzz/fuzz_bootctl.c @@ -3,25 +3,84 @@ /** * @file fuzz_bootctl.c - * @brief libFuzzer harness for boot control block (BCB) parsing + * @brief libFuzzer harness for boot control block loading * - * Feeds arbitrary data into the boot control block parser, exercising - * magic-number validation, slot metadata decoding, retry counters, - * and CRC integrity checks. + * The BCB comes out of flash, so every field in it is untrusted. This stages + * fuzzer bytes as that flash region and drives eos_bootctl_load(), then the + * state transitions a boot makes on whatever it managed to parse. + * + * This harness previously declared eos_bootctl_parse(), which has never + * existed in this tree -- eos_bootctl_load() is the real entry point. It + * compiled, because a declaration costs nothing, and it never linked, because + * EBLDR_BUILD_FUZZ defaults OFF and no CI job set it. */ +#include "eos_bootctl.h" +#include "eos_hal.h" + #include #include +#include -/* Forward-declare boot control block parser */ -extern int eos_bootctl_parse(const void *bcb_data, size_t bcb_len); +#define SIM_FLASH_BASE 0x08000000U +#define SIM_FLASH_SIZE (8u * 1024u) +static uint8_t sim_flash[SIM_FLASH_SIZE]; + +static int sim_flash_read(uint32_t addr, void *buf, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memcpy(buf, sim_flash + off, len); + return EOS_OK; +} -int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { - if (size < 8) { +static int sim_flash_write(uint32_t addr, const void *buf, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memcpy(sim_flash + off, buf, len); + return EOS_OK; +} + +static int sim_flash_erase(uint32_t addr, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memset(sim_flash + off, 0xFF, len); + return EOS_OK; +} + +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + if (size < 4) { return 0; } - eos_bootctl_parse(data, size); + static eos_board_ops_t ops; + memset(&ops, 0, sizeof ops); + ops.flash_read = sim_flash_read; + ops.flash_write = sim_flash_write; + ops.flash_erase = sim_flash_erase; + eos_hal_init(&ops); + + memset(sim_flash, 0xFF, sizeof sim_flash); + memcpy(sim_flash, data, size < SIM_FLASH_SIZE ? size : SIM_FLASH_SIZE); + + eos_bootctl_t bctl; + memset(&bctl, 0, sizeof bctl); + + if (eos_bootctl_load(&bctl) != EOS_OK) { + return 0; + } + /* Whatever it accepted, a boot then walks these transitions over it. */ + (void)eos_bootctl_increment_attempts(&bctl); + (void)eos_bootctl_set_pending(&bctl, EOS_SLOT_B); + (void)eos_bootctl_clear_pending(&bctl); + (void)eos_bootctl_reset_attempts(&bctl); + (void)eos_bootctl_save(&bctl); return 0; } diff --git a/tests/fuzz/fuzz_fw_update.c b/tests/fuzz/fuzz_fw_update.c index 7269a79..4b271de 100644 --- a/tests/fuzz/fuzz_fw_update.c +++ b/tests/fuzz/fuzz_fw_update.c @@ -3,41 +3,96 @@ /** * @file fuzz_fw_update.c - * @brief libFuzzer harness for firmware update stream processing + * @brief libFuzzer harness for the firmware update ingest path * - * Simulates a chunked firmware-update data stream, feeding arbitrary data - * into the update parser to exercise header validation, chunk sequencing, - * checksum verification, and boundary conditions. + * Drives a fuzzer-chosen byte stream through the real update API in the + * order a transport would: begin -> write (in varying chunk widths) -> + * finalize. The chunk widths come from the input too, so a header split + * across two writes is reachable. + * + * This harness previously declared eos_fw_update_init(), + * eos_fw_update_process_chunk() and eos_fw_update_finalize(void) -- an API + * that has never existed in this tree. It compiled, because a declaration + * costs nothing, and it never linked, because EBLDR_BUILD_FUZZ defaults OFF + * and no CI job set it. Same shape as eos#50's fuzz_devicetree, which + * declared an eos_dtb_parse() that was equally imaginary. */ +#include "eos_fw_update.h" +#include "eos_hal.h" + #include #include #include -/* Forward-declare firmware update APIs */ -extern int eos_fw_update_init(void); -extern int eos_fw_update_process_chunk(const uint8_t *chunk, size_t chunk_len); -extern int eos_fw_update_finalize(void); +/* A simulated flash large enough for the slot the update targets. */ +#define SIM_FLASH_BASE 0x08000000U +#define SIM_FLASH_SIZE (64u * 1024u) +static uint8_t sim_flash[SIM_FLASH_SIZE]; + +static int sim_flash_read(uint32_t addr, void *buf, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memcpy(buf, sim_flash + off, len); + return EOS_OK; +} + +static int sim_flash_write(uint32_t addr, const void *buf, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memcpy(sim_flash + off, buf, len); + return EOS_OK; +} + +static int sim_flash_erase(uint32_t addr, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memset(sim_flash + off, 0xFF, len); + return EOS_OK; +} -int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { - if (size < 4) { +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + if (size < 2) { return 0; } - eos_fw_update_init(); + static eos_board_ops_t ops; + memset(&ops, 0, sizeof ops); + ops.flash_read = sim_flash_read; + ops.flash_write = sim_flash_write; + ops.flash_erase = sim_flash_erase; + eos_hal_init(&ops); + memset(sim_flash, 0xFF, sizeof sim_flash); + + eos_fw_update_ctx_t ctx; + memset(&ctx, 0, sizeof ctx); - /* Feed data in variable-sized chunks derived from the fuzzer input */ + if (eos_fw_update_begin(&ctx, EOS_SLOT_B) != EOS_OK) { + return 0; + } + + /* Chunk widths come from the input, so a header straddling two writes + * is reachable -- that boundary is where a streaming parser goes wrong. */ size_t offset = 0; while (offset < size) { - size_t chunk_sz = (data[offset] % 64) + 1; - if (offset + chunk_sz > size) { - chunk_sz = size - offset; + size_t chunk = (size_t)(data[offset] % 64u) + 1u; + if (offset + chunk > size) { + chunk = size - offset; + } + if (eos_fw_update_write(&ctx, data + offset, chunk) != EOS_OK) { + break; } - eos_fw_update_process_chunk(data + offset, chunk_sz); - offset += chunk_sz; + offset += chunk; } - eos_fw_update_finalize(); - + (void)eos_fw_update_finalize(&ctx, EOS_UPGRADE_TEST); + eos_fw_update_abort(&ctx); return 0; } diff --git a/tests/fuzz/fuzz_image_verify.c b/tests/fuzz/fuzz_image_verify.c index 514f15f..9dd1d0e 100644 --- a/tests/fuzz/fuzz_image_verify.c +++ b/tests/fuzz/fuzz_image_verify.c @@ -14,20 +14,52 @@ #include #include -/* Forward-declare the image header parser */ -extern int eos_image_parse_header(const void *flash_base, size_t flash_len); +#include "eos_image.h" +#include "eos_hal.h" + +/* The declaration here used to be + * extern int eos_image_parse_header(const void *flash_base, size_t len); + * while the real one is + * int eos_image_parse_header(uint32_t addr, eos_image_header_t *out); + * Two pointers passed where an address and an out-parameter are expected. C + * has no overloading, so the symbol matched and this would have linked and + * run, writing a parsed header through whatever `size` happened to be. The + * header is included now, so the compiler checks the call instead. */ /** * Simulated flash read-back: the fuzzer data is treated as raw flash content * starting at offset 0. This lets the parser exercise its flash-pointer * arithmetic on arbitrary byte sequences. */ +#define SIM_FLASH_BASE 0x08000000U +#define SIM_FLASH_SIZE (16u * 1024u) +static uint8_t sim_flash[SIM_FLASH_SIZE]; + +static int sim_flash_read(uint32_t addr, void *buf, size_t len) +{ + if (addr < SIM_FLASH_BASE) return EOS_ERR_INVALID; + uint32_t off = addr - SIM_FLASH_BASE; + if ((uint64_t)off + len > SIM_FLASH_SIZE) return EOS_ERR_INVALID; + memcpy(buf, sim_flash + off, len); + return EOS_OK; +} + int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { + static eos_board_ops_t ops; + memset(&ops, 0, sizeof ops); + ops.flash_read = sim_flash_read; + eos_hal_init(&ops); + if (size < 4) { return 0; } - eos_image_parse_header(data, size); + /* Stage the fuzzer bytes as flash and parse from an address, which is + * what the real caller does. */ + memcpy(sim_flash, data, size < SIM_FLASH_SIZE ? size : SIM_FLASH_SIZE); + + eos_image_header_t hdr; + (void)eos_image_parse_header(SIM_FLASH_BASE, &hdr); return 0; } diff --git a/tests/fuzz/fuzz_recovery_protocol.c b/tests/fuzz/fuzz_recovery_protocol.c index 77b4cbc..9b6283f 100644 --- a/tests/fuzz/fuzz_recovery_protocol.c +++ b/tests/fuzz/fuzz_recovery_protocol.c @@ -3,24 +3,62 @@ /** * @file fuzz_recovery_protocol.c - * @brief libFuzzer harness for recovery protocol packet parsing + * @brief libFuzzer harness for the recovery write-range check * - * Exercises the recovery-mode packet parser with arbitrary byte streams, - * targeting framing, command dispatch, and length-field validation. + * eos_recovery_write_in_range() is the bounds decision the UART recovery + * path makes before writing attacker-supplied bytes into a slot. Every + * argument here comes from the fuzzer, including the slot geometry, so the + * wrap cases the function documents -- a zero base or size, a zero length, + * a write running past the slot, and base + offset overflowing -- are all + * reachable. + * + * This harness previously declared eos_recovery_parse_packet(), which has + * never existed in this tree. It compiled, because a declaration costs + * nothing, and it never linked, because EBLDR_BUILD_FUZZ defaults OFF and no + * CI job set it. + * + * eos_recovery_enter() is deliberately not driven: it is a command loop over + * a real UART that does not return on success, which is not a shape libFuzzer + * can work with. The range check is the part of recovery that takes untrusted + * numbers and answers a safety question, so it is the part worth fuzzing. */ +#include "eos_recovery.h" + #include #include +#include -/* Forward-declare recovery protocol handler */ -extern int eos_recovery_parse_packet(const uint8_t *pkt, size_t pkt_len); - -int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { - if (size < 2) { +int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) +{ + /* base(4) + slot_size(4) + offset(4) + len(2) */ + if (size < 14) { return 0; } - eos_recovery_parse_packet(data, size); + uint32_t base, slot_size, offset; + uint16_t len; + memcpy(&base, data, sizeof base); + memcpy(&slot_size, data + 4, sizeof slot_size); + memcpy(&offset, data + 8, sizeof offset); + memcpy(&len, data + 12, sizeof len); + + int rc = eos_recovery_write_in_range(base, slot_size, offset, len); + + /* The contract: a write it accepts must actually fit. If the function + * ever says yes to one that does not, that is the bug this harness is + * looking for, and it is worth an abort rather than a silent pass. */ + if (rc == EOS_OK) { + if (slot_size == 0 || len == 0) { + __builtin_trap(); + } + if ((uint64_t)base + offset + len > (uint64_t)base + slot_size) { + __builtin_trap(); + } + if ((uint64_t)base + offset < base) { + __builtin_trap(); + } + } return 0; }