Skip to content

fix(store): clamp ADR render cursor to prevent stack overflow#1175

Open
SEPURI-SAI-KRISHNA wants to merge 1 commit into
DeusData:mainfrom
SEPURI-SAI-KRISHNA:fix/adr-render-overflow
Open

fix(store): clamp ADR render cursor to prevent stack overflow#1175
SEPURI-SAI-KRISHNA wants to merge 1 commit into
DeusData:mainfrom
SEPURI-SAI-KRISHNA:fix/adr-render-overflow

Conversation

@SEPURI-SAI-KRISHNA

Copy link
Copy Markdown

What does this PR do?

Fixes a stack out-of-bounds write (CWE-787) in adr_render_section
(src/store/store.c), the helper cbm_adr_render uses to concatenate ADR
sections into a fixed 16 KB stack buffer (buf[ST_MAX_DEGREE * ST_GROWTH]).

The cursor was advanced by the return value of snprintf:

pos += snprintf(buf + pos, buf_sz - pos, "\n\n");
pos += snprintf(buf + pos, buf_sz - pos, "## %s\n%s", key, value);

snprintf returns the number of bytes it would have written, so once the
accumulated sections exceed buf_sz, pos moves past the end. On the next
section buf + pos points past the buffer and buf_sz - pos (an int
subtraction) goes negative, converting to a huge size_t — so snprintf
writes out of bounds.

The fix clamps pos into [0, buf_sz - 1] after every write, so each
snprintf receives a positive size and the returned cursor never escapes the
buffer. This mirrors the clamping already done by http_appendf in
src/ui/http_server.c and the sibling accumulators in store.c
(search_build_exclude_labels, bfs_build_types_clause).

Reproduced first (ASan, before the fix):

==ERROR: AddressSanitizer: stack-buffer-overflow ... WRITE of size 3
    #3 adr_render_section src/store/store.c:7008
    #4 cbm_adr_render     src/store/store.c:7057
    #5 test_adr_render_oversized_sections_no_overflow tests/test_store_arch.c
SUMMARY: AddressSanitizer: stack-buffer-overflow ... in vsnprintf

After the fix the full store_arch suite (53 tests) passes with no sanitizer
errors.

Adds a regression test adr_render_oversized_sections_no_overflow that sizes
three sections so the cursor lands exactly buf_sz + 8 after the second
section — the third section then performs the first out-of-bounds write right
at the buffer tail (ASan redzone), making the failure deterministic.

Note on scope

cbm_adr_render / cbm_store_adr_update_sections are exported in store.h
and exercised by the test suite, but currently have no production caller — the
manage_adr MCP tool and the UI /api/adr endpoint both persist raw content
via cbm_store_adr_store. So this is a latent defect in exported API rather
than a live remote overflow; the fix hardens the renderer before anything
wires up the section-merge path.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

Signed-off-by: SEPURI-SAI-KRISHNA <saik20533@gmail.com>
@DeusData DeusData added bug Something isn't working stability/performance Server crashes, OOM, hangs, high CPU/memory security Security vulnerabilities, hardening labels Jul 19, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 19, 2026
@DeusData DeusData added the priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. label Jul 19, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for the reproduce-first hardening fix. Triage is complete: this is a high-priority 0.9.1-rc stability/security item. Exact-head review at 13d523de484381cdbaff6fa52549836983cc88f0 found the change scoped to the exported ADR renderer and its deterministic ASan regression, with no new public surface or supply-chain changes. It remains in the maintainer review queue; this triage pass does not merge PRs.

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

Labels

bug Something isn't working priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. security Security vulnerabilities, hardening stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants