Harden shared image slot boundary validation

[Mohammed18-19]

Summary

• Add a shared eos_image_fits_slot() helper for complete image boundary validation.
• Validate the image header, payload, and TLV area against the configured flash slot.
• Use overflow-safe sequential size checks.
• Apply the shared validation across secure boot, slot management, recovery, and application loading paths.
• Add regression coverage for images and TLVs crossing the slot boundary.

Compatibility

eos_secure_boot_config_t now requires slot_size to be set to the flash capacity available to the image.

Existing in-tree callers have been updated accordingly. Out-of-tree integrations that zero-initialize eos_secure_boot_config_t must also set slot_size; otherwise secure boot will fail closed with EOS_SBOOT_ERR_BAD_HEADER.

Testing

Full host test execution is currently blocked by a pre-existing duplicate eboot_test_fdt_loader / test_fdt_loader target in tests/CMakeLists.txt on master.

The duplicate target causes CMake configuration to fail before the project can be configured and built with this PR, so the new tests have not yet been verified by a clean configure/build/ctest run.

The duplicate CMake target is unrelated to this change and should be removed in a separate PR against master.

Previously observed host build failures in core/ed25519_verify.c and core/sha512.c are also pre-existing baseline issues and are not modified by this PR.

[srpatcha]

Review — eBoot#109 "Harden shared image slot boundary validation"

head: d693147 author: Mohammed18-19 ci: fail

Verdict: Consolidates four copies of the slot-bounds check into one eos_image_fits_slot() helper and extends it to cover tlv_len — the right direction for §5.1's "minimal and auditable" TCB — but it adds a required field to a public config struct without updating the in-tree callers that zero-initialise it, so eos_secure_boot() now fails closed for every caller that has not been recompiled, and the PR's testing claim cannot have covered the diff.

Findings

#	Severity	File:line	Finding	Recommended fix
1	High	include/eos_secure_boot.h:31, core/secure_boot.c:90	slot_size is a new required field of the public eos_secure_boot_config_t. Callers using the documented memset(&cfg, 0, sizeof(cfg)) pattern get slot_size == 0; eos_image_fits_slot() then returns false for any header with hdr_size > 0, so eos_secure_boot() returns EOS_SBOOT_ERR_BAD_HEADER where it previously returned EOS_SBOOT_OK. tests/unit/test_secure_boot_policy.c:183,203,222,259 all zero-init and never set the field; three of those tests assert EOS_SBOOT_OK and will now fail. The PR body says nothing about compatibility impact (brief §8) — silence about a breaking change to a public struct is itself the finding.	Set cfg.slot_size in every in-tree caller, starting with the four sites in test_secure_boot_policy.c. State the compatibility impact and migration step in the PR body: any out-of-tree board integration that zero-initialises this struct stops booting until it sets the field. Failing closed is correct; doing it silently is not.
2	High	PR body, "Testing"	The body claims ctest --test-dir build --output-on-failure and "19/19 existing tests passed" while the same body states that full CMake reconfiguration is blocked. A build tree that was never reconfigured cannot contain core/image_slot_bounds.c or the two new tests, so that run did not exercise this diff. Per .ai/reviewer.md, an unsupported "verified" is the finding.	Remove the claim or replace it with output from a configure+build+ctest --no-tests=error run that actually includes the new sources. Until finding 3 is cleared, state plainly that the new tests have not run.
3	High	tests/CMakeLists.txt:187-191 and :201-205 (on origin/master)	Six required checks are red — Host Build & Tests, Build & Test (Linux x86_64), CI Gate, Cross-compile STM32F4, Fuzz Harness Build, Analyze (C/C++). Root cause is a pre-existing duplicate block on master: add_executable(eboot_test_fdt_loader ...) / add_test(NAME test_fdt_loader ...) appear twice, so CMake configure aborts with add_executable cannot create target "eboot_test_fdt_loader" because another target with the same name already exists before anything compiles. Not introduced here — the author correctly identified it — but it means nothing in this PR has been verified by CI either.	Delete the duplicate block (lines 201-205 plus its comment) on master in its own PR; this PR then rebases and gets real CI. No open eBoot PR currently covers this.
4	Medium	tests/unit/test_jump_app_bounds.c:184, :41	Assertion weakened: ASSERT(payload_bytes_read == image_size) became >=, and the accounting window at line 41 was simultaneously widened from addr < SLOT_B_ADDR to addr < SLOT_A_ADDR + SLOT_A_SIZE. The test no longer pins how much of the payload is read; it now passes for any over-read that stays inside slot A. A loosened assertion is a finding regardless of the reason given (.ai/reviewer.md).	If the intent is to also count TLV reads, assert the exact expected total (image_size + hdr.tlv_len) rather than relaxing the comparison.
5	Medium	tests/unit/test_secure_boot.c:202, :226	test_tlv_beyond_slot_is_rejected is a plain static void invoked directly from main(), not declared with the file's TEST() macro. Consequences: it never increments tests_run/tests_passed, so the suite's own return tests_passed == tests_run gate does not cover it; and it never calls eos_hal_init(&sim_ops), so it only works because a preceding run_test_* left the HAL registered — write_image() calls eos_crc32(), which needs a live HAL. Reordering or removing an earlier test silently breaks it. This is the exact failure the file's own closing comment was written to prevent.	Declare it as TEST(test_tlv_beyond_slot_is_rejected) and call run_test_tlv_beyond_slot_is_rejected() from main().
6	Low	core/secure_boot.c:90	This is the only one of four call sites without a preceding slot_size == 0 guard (core/recovery.c:318, core/slot_manager.c:54, stage1/jump_app.c:38 all have one). eos_image_fits_slot() returns true for an all-zero header against slot_size == 0, so the helper alone does not make the guard redundant.	Either add `cfg->slot_size == 0
7	Low	core/image_slot_bounds.c:1	Missing the // SPDX-License-Identifier: MIT / copyright / standards header that every other file in core/ carries (e.g. core/slot_manager.c:1-3). Relevant to the SPDX SBOM commitment in .github/STANDARDS.md.	Add the three-line header used by the sibling files.

Architecture conformance

Conforms. Master design §5.1: the new code sits in eBoot/core and eBoot/include, includes only eos_image.h, and adds no dependency pointing up a tier — core/image_slot_bounds.c is reachable from stage1/ and core/ only. Per .ai/architect.md's target shape, core/ is the correct home for shared boot logic and include/ for the contract. §5.1's "eBoot keeps the trusted computing base minimal and auditable" is served rather than harmed: replacing four hand-rolled copies of the same arithmetic with one non-overflowing helper is a net reduction in auditable surface, and extending it to tlv_len closes a real gap — the previous checks bounded header+payload only, so a manifest could declare a TLV region running past the slot. §8.1 ("Verify Manifest → Verify Image") is unaffected in ordering; the new gate runs before integrity streaming, which is the correct position. No tier or repository-placement question arises (§21: eBoot is Tier 1).

Proposed changes

Smallest sequence that keeps everything building:

1. Separate PR against master, nothing else in it — delete the duplicated test_fdt_loader block:

   --- a/tests/CMakeLists.txt
   @@ -199,7 +199,3 @@
   -# --- test_fdt_loader: device tree parsing against malformed blobs ---
   -
   -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)

   Verify with cmake -B build/host -DEBLDR_BUILD_TESTS=ON then ctest --test-dir build/host --output-on-failure --no-tests=error.
2. Rebase this PR on that, then set cfg.slot_size in all four test_secure_boot_policy.c fixtures (FLASH_BASE slot capacity, matching stage_bootable_image()).
3. Convert the new test at tests/unit/test_secure_boot.c:202 to the TEST() macro and call the generated run_test_* from main().
4. Restore an exact assertion at test_jump_app_bounds.c:184.
5. Re-run configure + build + ctest --no-tests=error and replace the PR body's Testing section with that output. Add the compatibility note for the eos_secure_boot_config_t field.

Items 6 and 7 are one-line cleanups that can ride along.

Not checked

• No build or test was run locally. The duplicate-target defect makes cmake configure fail before compilation, and this pipeline does not check out PR branches into the user's tree. Findings 1, 4, 5, 6 are from reading the head revision d693147, not from observed test output. Finding 1's specific claim — that the three EOS_SBOOT_OK assertions in test_secure_boot_policy.c now fail — is inferred from the code path (memset → slot_size == 0 → hdr_size > 0 → false), not observed.
• Cross-compile behaviour on STM32F4 and the ARM Cortex-M4 target: those jobs are red or skipped for the same configure failure, so no target-side evidence exists.
• Fuzz coverage for the new tlv_len path — .ai/security.md requires fuzz, not only unit tests, for externally reachable parsers. The Fuzz Harness Build job is red, so whether any harness reaches eos_image_fits_slot() is unknown.
• Out-of-tree consumers of eos_secure_boot_config_t. Only this repository was searched; board integrations living elsewhere were not.
• Whether eos_image_parse_header() can return EOS_OK for a header with hdr_size == 0, which is what would make finding 6 reachable in practice.

---

Automated architecture review of d693147bccc2 — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.