diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 06fbf1d..c65c144 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,7 +62,7 @@ python scripts/generate_config.py configs/example_boot.yaml /tmp/generated/ ## Test Suites -eboot includes 17 C unit test suites that run natively on the host, plus a +eboot includes 19 C unit test suites that run natively on the host, plus a set of Python tests that check the build files and tooling statically: | Test | Covers | @@ -70,6 +70,7 @@ set of Python tests that check the build files and tooling statically: | `test_bootctl` | Boot control block save/load, CRC, rollback | | `test_crypto` | SHA-256 against known vectors | | `test_image_verify` | Image header parse bounds | +| `test_image_abi` | Firmware image header ABI compatibility | | `test_recovery` | Recovery-mode UART protocol handler | | `test_slot_size_bounds` | `verify_slot()` rejects `image_size` larger than the slot | | `test_fw_transport` | UART raw / XMODEM / YMODEM firmware transport framing | @@ -84,6 +85,7 @@ set of Python tests that check the build files and tooling statically: | `test_keystore` | Boot keystore management | | `test_rollback` | Anti-rollback security counter | | `test_storage` | Unified storage bounds checking | +| `test_ecc` | ECC scrub-region and request range validation | Every `tests/unit/test_*.c` suite must be registered in `tests/CMakeLists.txt` with both an `add_executable()` and an `add_test()`. A suite that is not diff --git a/TASKS.md b/TASKS.md index e753663..3e17834 100644 --- a/TASKS.md +++ b/TASKS.md @@ -23,6 +23,7 @@ Status is one of: `todo`, `in-progress`, `blocked`, `review`, `done`. | T-004 | CI ran zero tests and reported success | testing | reviewer | `ci.yml` configured with `-DBUILD_TESTS=ON`, but this project's option is `EBLDR_BUILD_TESTS`; the flag set an unrelated cache variable and no test was ever built. `ctest` exits 0 when it finds no tests (verified: exit code 0), so the job passed green. This is how T-001 shipped. Fixed the flag and added `--no-tests=error`; with the old flag the job now exits 8. CI runs 11 tests. | | T-005 | Fix a mismatched `extern` that no compiler could see | backend | reviewer | `core/secure_boot.c` declared `eos_ed25519_verify(msg, msg_len, sig, pubkey)` while the definition is `(signature, public_key, message, msg_len)` — a caller would have passed a `size_t` length where a key pointer was expected. Both ad-hoc `extern`s removed; the single prototype now lives in `include/eos_crypto_boot.h`. | | T-006 | Build the two source files that were never compiled | backend | reviewer | `core/secure_boot.c` and `core/fdt_loader.c` were absent from `CMakeLists.txt`. `secure_boot.c` also called `eos_sha256()`, which was defined nowhere in the tree, so it could not have linked. Added the one-shot `eos_sha256()` to `core/crypto_boot.c` and both files to the build; clean under `-Wall -Wextra`. | +| T-007 | Make ECC region bounds checks overflow-safe | security | independent reviewer | Focused CTest: **1/1 passed**; the same test suite reports both overflow assertions failing against the old implementation. Full Release C suite: **19/19 passed**. Boundary tests cover wrapped initialization, wrapped requests, the exclusive end, and ordinary out-of-range requests. | --- diff --git a/core/ecc_scrub.c b/core/ecc_scrub.c index 57734e3..1e6a724 100644 --- a/core/ecc_scrub.c +++ b/core/ecc_scrub.c @@ -14,6 +14,7 @@ int eos_ecc_init(eos_ecc_ctx_t *ctx, uint32_t base, uint32_t size) { if (!ctx) return -1; + if (size > UINT32_MAX - base) return -1; memset(ctx, 0, sizeof(*ctx)); ctx->base_addr = base; ctx->size_bytes = size; @@ -43,8 +44,11 @@ int eos_ecc_scrub(eos_ecc_ctx_t *ctx) int eos_ecc_check_region(eos_ecc_ctx_t *ctx, uint32_t addr, uint32_t len) { if (!ctx) return -1; - if (addr < ctx->base_addr || (addr + len) > (ctx->base_addr + ctx->size_bytes)) return -1; - if (addr < ctx->base_addr || addr + len > ctx->base_addr + ctx->size_bytes) return -1; + if (addr < ctx->base_addr) return -1; + + uint32_t offset = addr - ctx->base_addr; + if (offset > ctx->size_bytes || len > ctx->size_bytes - offset) return -1; + volatile uint32_t *p = (volatile uint32_t *)(uintptr_t)addr; uint32_t words = len / 4; diff --git a/include/eos_ecc.h b/include/eos_ecc.h index 19709e7..c8b9ba0 100644 --- a/include/eos_ecc.h +++ b/include/eos_ecc.h @@ -28,8 +28,11 @@ typedef struct { int scrub_complete; } eos_ecc_ctx_t; +/* Initialize a non-wrapping 32-bit ECC memory range. */ int eos_ecc_init(eos_ecc_ctx_t *ctx, uint32_t base, uint32_t size); int eos_ecc_scrub(eos_ecc_ctx_t *ctx); +/* Check that [addr, addr + len) is contained in the configured range, then + * read each complete word to trigger the platform's ECC machinery. */ int eos_ecc_check_region(eos_ecc_ctx_t *ctx, uint32_t addr, uint32_t len); void eos_ecc_dump(const eos_ecc_ctx_t *ctx); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 135254a..3bf1d23 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,6 +91,11 @@ add_executable(eboot_test_storage unit/test_storage.c) target_link_libraries(eboot_test_storage PRIVATE eboot_core) add_test(NAME test_storage COMMAND eboot_test_storage) +# --- test_ecc: ECC memory range validation --- +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) + # --- Valgrind test targets --- find_program(VALGRIND valgrind) if(VALGRIND) diff --git a/tests/unit/test_ecc.c b/tests/unit/test_ecc.c new file mode 100644 index 0000000..06f7cd4 --- /dev/null +++ b/tests/unit/test_ecc.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project + +/** + * @file test_ecc.c + * @brief Unit tests for overflow-safe ECC memory range validation + * + * Only zero-length check requests are used here. They exercise every range + * boundary without dereferencing the synthetic 32-bit addresses on the host. + */ + +#include "eos_ecc.h" + +#include +#include + +static int failures; + +#define CHECK(condition) \ + do { \ + if (!(condition)) { \ + fprintf(stderr, "[FAIL] %s:%d: %s\n", \ + __FILE__, __LINE__, #condition); \ + failures++; \ + } \ + } while (0) + +static void test_init_rejects_wrapped_range(void) +{ + eos_ecc_ctx_t ctx; + + CHECK(eos_ecc_init(NULL, 0, 4096) == -1); + CHECK(eos_ecc_init(&ctx, UINT32_MAX - 0xFFu, 0x100u) == -1); + CHECK(eos_ecc_init(&ctx, UINT32_MAX - 0xFFu, 0xFFu) == 0); +} + +static void test_check_rejects_wrapped_request(void) +{ + eos_ecc_ctx_t ctx; + CHECK(eos_ecc_init(&ctx, 0x1000u, 0x1000u) == 0); + + /* UINT32_MAX + 2 wrapped to 1 in the old addr + len comparison. len is + * below one word so a buggy implementation returns without dereferencing. */ + CHECK(eos_ecc_check_region(&ctx, UINT32_MAX, 2) == -1); +} + +static void test_check_preserves_boundary_semantics(void) +{ + eos_ecc_ctx_t ctx; + CHECK(eos_ecc_init(&ctx, 0x1000u, 0x1000u) == 0); + + CHECK(eos_ecc_check_region(NULL, 0x1000u, 0) == -1); + CHECK(eos_ecc_check_region(&ctx, 0x0FFFu, 0) == -1); + CHECK(eos_ecc_check_region(&ctx, 0x1000u, 0) == 0); + CHECK(eos_ecc_check_region(&ctx, 0x2000u, 0) == 0); + CHECK(eos_ecc_check_region(&ctx, 0x2001u, 0) == -1); + CHECK(eos_ecc_check_region(&ctx, 0x1FFFu, 2) == -1); +} + +int main(void) +{ + test_init_rejects_wrapped_range(); + test_check_rejects_wrapped_request(); + test_check_preserves_boundary_semantics(); + + if (failures != 0) { + fprintf(stderr, "%d ECC test(s) failed\n", failures); + return 1; + } + + printf("ECC range validation tests passed\n"); + return 0; +}