From a6bca1b4ff77a0c05077125b5ae61137087874cc Mon Sep 17 00:00:00 2001 From: Shrenik Mensinkai Date: Sun, 30 Aug 2026 00:43:56 +0530 Subject: [PATCH] fix(recovery): reject out-of-range UART writes via the existing helper The write handler redeclared slot size and skipped eos_recovery_write_in_range, so a wire-controlled offset could miss wrap and zero-base checks. Use the helper once and cover those cases in the host tests. --- core/recovery.c | 10 +++++----- include/eos_recovery.h | 37 +++++++++++++++++++++++++++++++++++++ tests/unit/test_recovery.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 include/eos_recovery.h diff --git a/core/recovery.c b/core/recovery.c index 18a88ba..b522773 100644 --- a/core/recovery.c +++ b/core/recovery.c @@ -15,6 +15,7 @@ */ #include "eos_bootctl.h" +#include "eos_recovery.h" #include "eos_image.h" #include "eos_hal.h" #include "eos_crypto_boot.h" @@ -281,11 +282,10 @@ static int recovery_handle_write(eos_slot_t slot, uint32_t offset, uint16_t len) if (len > sizeof(buf)) return recovery_send_nack(); - /* offset/len come straight from the wire; without this check a - * recovery client can write past the slot boundary into the other - * slot, boot-control blocks, or the boot log. */ - uint32_t slot_size = eos_hal_slot_size(slot); - if (slot_size == 0 || (uint64_t)offset + len > (uint64_t)slot_size) + /* offset/len come straight from the wire. The helper rejects a + * zero slot base, wrap of base+offset, and writes that run past + * the slot into boot-control, the other slot, or the boot log. */ + if (eos_recovery_write_in_range(base, slot_size, offset, len) != EOS_OK) return recovery_send_nack(); recovery_send_ack(); diff --git a/include/eos_recovery.h b/include/eos_recovery.h new file mode 100644 index 0000000..33d7011 --- /dev/null +++ b/include/eos_recovery.h @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 EoS Project +// ISO/IEC 25000 | ISO/IEC/IEEE 15288:2023 + +/** + * @file eos_recovery.h + * @brief UART recovery mode entry and write-range checks + */ + +#ifndef EOS_RECOVERY_H +#define EOS_RECOVERY_H + +#include "eos_bootctl.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief Return EOS_OK if a recovery write of @p len bytes at @p offset + * stays inside the slot at @p base. + * + * Rejects a zero base or slot size, a zero length, a write that runs + * past the slot, and wrap of @c base + offset. + */ +int eos_recovery_write_in_range(uint32_t base, uint32_t slot_size, + uint32_t offset, uint16_t len); + +/** + * @brief Enter the UART recovery command loop. Does not return on success. + */ +int eos_recovery_enter(eos_bootctl_t *bctl); + +#ifdef __cplusplus +} +#endif +#endif /* EOS_RECOVERY_H */ diff --git a/tests/unit/test_recovery.c b/tests/unit/test_recovery.c index a87e80d..f8aaa45 100644 --- a/tests/unit/test_recovery.c +++ b/tests/unit/test_recovery.c @@ -14,6 +14,7 @@ */ #include "eos_bootctl.h" +#include "eos_recovery.h" #include "eos_hal.h" #include "eos_crypto_boot.h" #include @@ -197,8 +198,6 @@ static void script_append(const uint8_t *bytes, size_t len) script_len += len; } -extern int eos_recovery_enter(eos_bootctl_t *bctl); - /* ---- Test harness ---- */ static int tests_run = 0; @@ -233,6 +232,32 @@ static void setup(void) eos_hal_init(&sim_ops); } +TEST(test_write_in_range_rejects_zero_base) +{ + ASSERT(eos_recovery_write_in_range(0, 0x1000, 0, 16) == EOS_ERR_INVALID); +} + +TEST(test_write_in_range_rejects_zero_len) +{ + ASSERT(eos_recovery_write_in_range(0x4000, 0x1000, 0, 0) == EOS_ERR_INVALID); +} + +TEST(test_write_in_range_rejects_past_slot) +{ + ASSERT(eos_recovery_write_in_range(0x4000, 0x1000, 0x1000, 16) == EOS_ERR_INVALID); +} + +TEST(test_write_in_range_rejects_base_offset_wrap) +{ + ASSERT(eos_recovery_write_in_range(0xFFFFFFF0u, 0x1000, 0x20, 16) == EOS_ERR_INVALID); +} + +TEST(test_write_in_range_accepts_in_slot) +{ + ASSERT(eos_recovery_write_in_range(0x4000, 0x1000, 0, 16) == EOS_OK); + ASSERT(eos_recovery_write_in_range(0x4000, 0x1000, 0x1000 - 16, 16) == EOS_OK); +} + /* Authenticate, then drive an out-of-bounds WRITE (offset+len past the end * of slot A) followed by a well-formed in-bounds WRITE. Before the fix, * recovery_handle_write() never validated `offset` against the slot size, @@ -295,6 +320,11 @@ TEST(test_write_rejects_offset_past_slot_end) int main(void) { printf("=== test_recovery ===\n"); + run_test_write_in_range_rejects_zero_base(); + run_test_write_in_range_rejects_zero_len(); + run_test_write_in_range_rejects_past_slot(); + run_test_write_in_range_rejects_base_offset_wrap(); + run_test_write_in_range_accepts_in_slot(); run_test_write_rejects_offset_past_slot_end(); printf("%d/%d tests passed\n", tests_passed, tests_run); return (tests_passed == tests_run) ? 0 : 1;