From 27901dd5fe6b135f9c236df50e81c8a42c932099 Mon Sep 17 00:00:00 2001 From: Swayam Nayak <154440440+swayam-2003@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:13:39 +0530 Subject: [PATCH] Harden recovery write/verify against slot boundary violations Use eos_recovery_write_in_range() in the WRITE handler to remove a duplicate variable that broke compilation, and reject VERIFY requests when the parsed image exceeds the slot capacity before integrity checks stream past the slot boundary. Signed-off-by: Swayam Nayak <154440440+swayam-2003@users.noreply.github.com> --- CHANGELOG.md | 1 + core/recovery.c | 6 ++++ tests/unit/test_recovery.c | 59 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0eaa82..2fb33e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - **`eos_image_parse_header`:** validates `hdr_version`, rejecting 0 and anything newer than this build understands. - **`tools/eos_sign.py`:** `SIG_TYPE_ED25519` was `1` — that is `EOS_SIG_CRC32` in `eos_types.h`, which `eos_image_verify_signature()` rejects outright — and `IMG_FLAG_SIGNED` was `1 << 2`, which is `EOS_IMG_FLAG_DEBUG`. It also never set `EOS_IMG_FLAG_HASH_SHA256`, so the bootloader read the stored SHA-256 as a CRC32. Constants now match `include/eos_types.h`. - **`recovery.c`:** UART recovery writes now reject offsets and lengths that leave the target slot, including wrap of `base + offset`. +- **`recovery.c`:** UART recovery VERIFY rejects image headers whose payload exceeds the slot capacity before streaming integrity reads past the slot boundary. - **`image_verify.c`:** `eos_image_parse_header` rejects a `load_addr + image_size` that overflows `uint32_t` instead of wrapping the runtime end address. - **`image_verify.c`:** The CRC32 integrity path now fails closed on a flash read error. `eos_crc32()` returned `0` when `eos_hal_flash_read()` failed, which is indistinguishable from a region that genuinely hashes to `0`, so an image whose payload could not be read passed `eos_image_verify_integrity()` when the stored CRC was `0`. The stored CRC lives in the unauthenticated header, so setting it to `0` is trivial. The SHA-256 path already propagated the read error; the two now behave the same. - **`image_verify.c`:** `eos_image_verify_integrity` rejects a zero `image_size`, and an `addr + hdr_size` that wraps `uint32_t`, instead of computing a payload address that is not the payload. diff --git a/core/recovery.c b/core/recovery.c index c36687f..b233504 100644 --- a/core/recovery.c +++ b/core/recovery.c @@ -314,6 +314,12 @@ static int recovery_handle_verify(eos_slot_t slot) if (rc != EOS_OK) return recovery_send_nack(); + uint32_t slot_size = eos_hal_slot_size(slot); + if (slot_size == 0 || + hdr.hdr_size > slot_size || + hdr.image_size > slot_size - hdr.hdr_size) + return recovery_send_nack(); + /* eos_image_verify_integrity() adds hdr_size internally — pass base addr only */ rc = eos_image_verify_integrity(&hdr, addr); if (rc != EOS_OK) diff --git a/tests/unit/test_recovery.c b/tests/unit/test_recovery.c index e77d3d9..e99133d 100644 --- a/tests/unit/test_recovery.c +++ b/tests/unit/test_recovery.c @@ -17,6 +17,7 @@ #include "eos_recovery.h" #include "eos_hal.h" #include "eos_crypto_boot.h" +#include "eos_image.h" #include #include #include @@ -37,10 +38,15 @@ static uint8_t sim_flash[SIM_FLASH_SIZE]; #define SIM_SLOT_B_ADDR 0x6000 #define SIM_SLOT_B_SIZE 0x1000 +static size_t verify_payload_bytes_read = 0; + static int sim_flash_read(uint32_t addr, void *buf, size_t len) { if (addr + len > SIM_FLASH_SIZE) return EOS_ERR_FLASH; memcpy(buf, &sim_flash[addr], len); + if (addr >= SIM_SLOT_A_ADDR + sizeof(eos_image_header_t) && + addr < SIM_SLOT_B_ADDR) + verify_payload_bytes_read += len; return EOS_OK; } @@ -171,6 +177,7 @@ static const eos_board_ops_t sim_ops = { /* ---- Recovery protocol packet (mirrors the private struct in recovery.c) ---- */ #define RCVR_CMD_WRITE 0x04 +#define RCVR_CMD_VERIFY 0x05 #define RCVR_CMD_AUTH 0x10 #define RCVR_ACK 0xAA #define RCVR_NACK 0x55 @@ -228,6 +235,7 @@ static void setup(void) script_len = 0; script_pos = 0; out_len = 0; + verify_payload_bytes_read = 0; eos_hal_init(&sim_ops); } @@ -314,11 +322,62 @@ TEST(test_write_rejects_offset_past_slot_end) ASSERT(memcmp(&sim_flash[SIM_SLOT_A_ADDR], payload, sizeof(payload)) == 0); } +static void fill_header(eos_image_header_t *hdr, uint32_t image_size) +{ + memset(hdr, 0, sizeof(*hdr)); + hdr->magic = EOS_IMG_MAGIC; + hdr->hdr_version = EOS_IMAGE_HDR_VERSION; + hdr->hdr_size = (uint16_t)sizeof(eos_image_header_t); + hdr->image_size = image_size; + hdr->load_addr = 0x20000000; + hdr->entry_addr = 0x20000100; +} + +/* VERIFY must reject an image whose payload exceeds the slot, instead of + * streaming reads past the slot boundary during eos_image_verify_integrity(). */ +TEST(test_verify_rejects_oversized_image_before_reading_payload) +{ + eos_image_header_t hdr; + fill_header(&hdr, SIM_SLOT_A_SIZE + 0x400u); + memcpy(&sim_flash[SIM_SLOT_A_ADDR], &hdr, sizeof(hdr)); + + eos_sha256_ctx_t ctx; + uint8_t auth_response[32]; + eos_sha256_init(&ctx); + eos_sha256_update(&ctx, SIM_CHALLENGE, sizeof(SIM_CHALLENGE)); + eos_sha256_update(&ctx, SIM_SHARED_SECRET, sizeof(SIM_SHARED_SECRET)); + eos_sha256_final(&ctx, auth_response); + + uint8_t pkt[8]; + put_pkt(pkt, RCVR_CMD_AUTH, 0, 0, 0); + script_append(pkt, sizeof(pkt)); + put_pkt(pkt, RCVR_CMD_AUTH, 0, 0, 0); + script_append(pkt, sizeof(pkt)); + script_append(auth_response, sizeof(auth_response)); + + put_pkt(pkt, RCVR_CMD_VERIFY, EOS_SLOT_A, 0, 0); + script_append(pkt, sizeof(pkt)); + + eos_bootctl_t bctl; + eos_bootctl_init_defaults(&bctl); + + if (setjmp(exit_jmp) == 0) { + eos_recovery_enter(&bctl); + } + + /* auth ACK then VERIFY NACK */ + ASSERT(out_len >= 35); + ASSERT(out_buf[33] == RCVR_ACK); + ASSERT(out_buf[34] == RCVR_NACK); + ASSERT(verify_payload_bytes_read == 0); +} + int main(void) { printf("=== test_recovery ===\n"); run_test_write_range_helper_rejects_invalid_bounds(); run_test_write_rejects_offset_past_slot_end(); + run_test_verify_rejects_oversized_image_before_reading_payload(); printf("%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1; }