Harden recovery VERIFY against slot boundary violations

[swayam-2003]

Summary

• WRITE handler: Removed a duplicate slot_size declaration that broke compilation; routed bounds checking through the existing eos_recovery_write_in_range() helper.
• VERIFY handler: Added the same slot-capacity guard used in slot_manager.c so UART recovery cannot stream integrity reads past a slot when image_size exceeds real slot capacity.

Approach

Reused the proven bounds check from boot-time slot verification (PR #50). Extended tests/unit/test_recovery.c with a VERIFY regression test.

Testing

cmake -B build -DEBLDR_BUILD_TESTS=ON
cmake --build build --parallel
ctest --test-dir build -R test_recovery --output-on-failure

DCO

Signed-off-by: Swayam Nayak 154440440+swayam-2003@users.noreply.github.com

[Copilot]

🟢 Approval recommended

The changes consistently apply proven slot-boundary checks to recovery WRITE/VERIFY paths and add a targeted regression test for the new VERIFY guard.

Pull request overview

This PR hardens the UART recovery protocol against out-of-bounds flash access by ensuring both WRITE and VERIFY operations respect the real, board-defined slot boundaries, preventing slot boundary violations during writes and integrity verification reads.

Changes:

• WRITE handler: routes bounds checking through the existing eos_recovery_write_in_range() helper (and removes the duplicate slot_size declaration that previously broke compilation).
• VERIFY handler: rejects headers whose declared payload would exceed the actual slot capacity before calling eos_image_verify_integrity().
• Unit tests: adds a regression test ensuring VERIFY rejects oversized images before any payload bytes are read; updates changelog accordingly.

File summaries

File	Description
core/recovery.c	Uses shared range-check helper for WRITE; adds slot-capacity guard to VERIFY to prevent out-of-slot integrity reads.
tests/unit/test_recovery.c	Adds a VERIFY regression test that asserts no payload bytes are read when the header’s payload exceeds slot capacity.
CHANGELOG.md	Documents the new recovery VERIFY slot-capacity rejection behavior.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[srpatcha]

Verified — the fix is right, and the conflict with #58 is one dropped line

offset and len arrive on the recovery UART and reach eos_hal_flash_write(base + offset, ...). Without a bound, a recovery client can write past the slot into the other slot, the boot-control blocks, or the boot log — from a serial port, on a device that is by definition already in trouble.

Reusing eos_recovery_write_in_range is the right call. That helper already exists on master and gets the arithmetic right:

/* Check len first so slot_size - len cannot underflow. */
if ((uint32_t)len > slot_size || offset > slot_size - (uint32_t)len)

Checking len before subtracting is the part that matters — offset + len > slot_size on 32-bit values is exactly the wrap that eos#87 is fixing in the OTA path this week.

The conflict

This does not build merged with #58, but the cause is mechanical, not a disagreement:

core/recovery.c:286: error: 'slot_size' undeclared

Both branches declare uint32_t slot_size = eos_hal_slot_size(slot); — yours at line 278, #58's at 286 — and both add a bounds check. The merge kept your call and #58's declaration line, then #58's own inline check went away with its declaration. One line is missing, not a design clash.

Resolving it is restoring that declaration:

uint32_t base = eos_hal_slot_addr(slot);
uint32_t slot_size = eos_hal_slot_size(slot);

Done that locally on top of #58: 0 build errors, ctest 16/16.

Which check survives

#58 inlines (uint64_t)offset + len > (uint64_t)slot_size; yours calls the existing helper. Both are correct. I would keep yours — the helper is already in the tree, it is already tested, and a second inline copy of the same bound is how the two drift. Worth saying so on #58 rather than deciding silently, so I have.

Blocker outside this PR

eBoot sets required_signatures: true on master and commits here are unsigned, as are every contributor's. Nothing in this repository is mergeable until that policy changes — I have raised it with the maintainer. Also note master does not build at all right now; #58 repairs it and needs to land first.

[Copilot]

🟢 Approval recommended

The added slot-capacity guard mirrors the established slot verification pattern and is backed by a focused unit regression test with no evident correctness issues in the diffs.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[swayam-2003]

Rebased onto latest upstream/master (ae1e0f7). Previous CI failures were from a stale base with upstream merge compile regressions (now fixed in #69). The WRITE bounds path is covered upstream (#74); this PR now adds only the VERIFY slot-capacity check before integrity reads + unit test.