Skip to content

fix: prevent offset+len overflow in flash log read#449

Open
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/nvm-flash-log-read-overflow
Open

fix: prevent offset+len overflow in flash log read#449
MarkAtwood wants to merge 1 commit into
wolfSSL:mainfrom
MarkAtwood:fix/nvm-flash-log-read-overflow

Conversation

@MarkAtwood

Copy link
Copy Markdown

Bug

In wh_NvmFlashLog_Read (src/wh_nvm_flash_log.c:690), the bounds check

if (offset + data_len > obj->meta.len)

adds two whNvmSize (uint16_t) values. On a platform where int is 16 bits (standards-permissible; C only guarantees int >= 16 bits), the addition wraps modulo 65536. A crafted offset=0xFFFF, data_len=11 against a 10-byte object wraps to 10, passes the check, and the subsequent memcpy(data, obj_data, data_len) at line 693 reads out of bounds.

On mainstream >=32-bit-int targets (e.g. ARM Cortex-M) integer promotion computes the sum in 32-bit int, so it does not wrap there — this is a latent portability / defense-in-depth defect for 16-bit-int embedded targets.

Fix

Force 32-bit arithmetic with explicit casts so the sum never wraps regardless of int width:

if ((uint32_t)offset + (uint32_t)data_len > obj->meta.len)

This mirrors the already-remediated sibling check in src/wh_server_nvm.c (which avoids the addition via subtraction).

Build verification

Compiled the changed translation unit on Ubuntu 24.04 / gcc (32-bit int) against the repo's own test config (test/config/wolfhsm_cfg.h, -DWOLFHSM_CFG) linked to a prebuilt wolfSSL install: gcc_exit=0, no warnings.

Reported by static analysis (Fenrir finding 389).

Copilot AI review requested due to automatic review settings July 9, 2026 23:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes a portability/security edge case in wh_NvmFlashLog_Read where offset + data_len could overflow on 16-bit-int targets, allowing an out-of-bounds read.

Changes:

  • Forces 32-bit arithmetic in the bounds check by casting offset and data_len to uint32_t prior to addition.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants