fix(ota): reject oversized chunks without wrapping the length add

[ShrenikMensinkai]

Summary

eos_ota_write_chunk takes a host-controlled length and used bytes_written + len > total_size. That add can wrap, so a huge chunk can look in-range, then SHA-256 is updated with that length. This PR compares against remaining space and adds host tests for overflow and wrap.

This addresses:

• Issue: integer wrap on the OTA write-chunk bound
• Approach: reject using remaining bytes, not a summed length
• Testing: new overflow and wrap tests; existing full-update path unchanged
• Limits: write-chunk bound only; no flash, transport, or slot-size vs expected_size; no hardware run on this machine

Type of Change

• feat — New feature
• fix — Bug fix
• docs — Documentation only
• style — Formatting, no code change
• refactor — Code restructuring without behavior change
• test — Add or fix tests
• build — Build system or dependency changes
• ci — CI/CD pipeline changes
• perf — Performance improvement

Changes

• Reject a chunk when len is larger than remaining update size, without adding the two values
• Added a host test that writes 32 then 33 bytes into a 64-byte update and checks the second write is rejected
• Added a host test that accepts one byte then rejects SIZE_MAX, so the old wrap cannot fail open

Testing

• Unit tests pass (ctest --test-dir build --output-on-failure)
• Integration tests pass
• Manual testing performed
• New tests added for new functionality

CMake and a C compiler were not available on the Windows host used for this change. Please run a native build with EOS_BUILD_TESTS=ON and ctest -R test_ota before merge.

Pre-Submission Checklist

• Code compiles without warnings (-Wall -Wextra -Werror for C)
• All existing tests pass
• New tests added for new functionality
• Documentation updated if API changed
• Commit messages follow <type>(<scope>): <description> convention
• Branch is rebased on latest master

Related Issues

None. Found while reviewing the OTA write path for the screening.

Screenshots / Logs

Not applicable. Host unit tests only; no board or OTA capture.

Additional Notes

• Only the write-chunk bound. Slot size vs expected_size, transport, and apply/reboot are unchanged.
• After a rejected chunk, bytes_received must not move (covered by the new tests).
• Tests are host-side; they do not programme flash.

[srpatcha]

Verified — and this one is invisible on the host, which is the interesting part

The old check adds before comparing:

if (g_ota.bytes_written + len > g_ota.total_size) return -1;

bytes_written is uint32_t and len is size_t. On a Cortex-M target both are 32 bits, the sum is computed in 32 bits, and it wraps. Demonstrated:

32-bit target semantics:
  bytes_written=100  total_size=64  len=0xFFFFFFC0
  bytes_written + len  = 36   (wrapped)
  old check (sum > total)     -> ACCEPT   <-- overflow
  new check (len > remaining) -> reject

64-bit host semantics (why it hides):
  bytes_written + len  = 4294967332  (no wrap)
  old check                   -> reject

An accepted oversized chunk goes straight into

eos_sha256_update(&g_ota.hash_ctx, data, len);

which reads len bytes from a caller-supplied pointer — an out-of-bounds read of nearly 4 GB, driven by a length field that arrives with the update. That is the OTA path, so the length is attacker-controlled by construction.

The host build cannot reproduce it. size_t is 64 bits there, the sum does not wrap, and the old check rejects correctly. Every test in this repository runs on the host. That is worth stating in the PR body, because it explains why a test alone would not have caught this and why the review argument matters.

Comparing against remaining space is the right idiom — no addition, so nothing to wrap. The bytes_written > total_size guard in front of the subtraction is necessary rather than defensive: without it the subtraction underflows and produces a huge remaining allowance, reintroducing the bug in the other direction.

The test

write_chunk(fw, 32) then write_chunk(fw, 33) against a 64-byte image, asserting the second is refused and bytes_received stays 32, tests the boundary correctly on the host. It does not reach the wrap — nothing on a 64-bit host can — and asserting bytes_received rather than only the return value is what makes it a real check: a rejection that had already advanced the counter would pass a return-value-only test.

Verification

Merged onto origin/master + #82: 0 build errors, ctest 28/28. Wrap behaviour confirmed with a standalone 32-bit-semantics reproduction, since the host build cannot show it. Not verified against master alone, which does not configure.