fix(fuzz): point the harnesses at functions that exist, and build them in CI

.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

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];

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");
 

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 <stdint.h>
 #include <stddef.h>
+#include <string.h>
 
-/* 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;
 }

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 <stdint.h>
 #include <stddef.h>
 #include <string.h>
 
-/* 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;
 }