diff --git a/src/store/store.c b/src/store/store.c index 7b966051a..31217f24f 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -7002,12 +7002,33 @@ cbm_adr_sections_t cbm_adr_parse_sections(const char *content) { return result; } -/* Append a section to the render buffer. */ +/* Append a section to the render buffer. + * + * snprintf returns the length it WOULD have written, so a section larger than + * the space left would otherwise push pos past buf_sz; the next call would then + * compute a wrapped (huge) remaining size from (buf_sz - pos) and write out of + * bounds. Clamp pos into [0, buf_sz-1] after every write so each snprintf gets + * a positive size and the returned cursor never escapes the buffer. */ static int adr_render_section(char *buf, int buf_sz, int pos, const char *key, const char *value) { + if (buf_sz <= 0) { + return 0; + } + if (pos < 0) { + pos = 0; + } + if (pos >= buf_sz) { + return buf_sz - SKIP_ONE; + } if (pos > 0) { - pos += snprintf(buf + pos, buf_sz - pos, "\n\n"); + pos += snprintf(buf + pos, (size_t)(buf_sz - pos), "\n\n"); + if (pos >= buf_sz) { + return buf_sz - SKIP_ONE; + } + } + pos += snprintf(buf + pos, (size_t)(buf_sz - pos), "## %s\n%s", key, value); + if (pos >= buf_sz) { + pos = buf_sz - SKIP_ONE; } - pos += snprintf(buf + pos, buf_sz - pos, "## %s\n%s", key, value); return pos; } diff --git a/tests/test_store_arch.c b/tests/test_store_arch.c index 777140aa9..159c07f95 100644 --- a/tests/test_store_arch.c +++ b/tests/test_store_arch.c @@ -741,6 +741,52 @@ TEST(adr_render_empty) { PASS(); } +/* Helper: allocate a NUL-terminated buffer of `len` 'x' characters. */ +static char *adr_test_fill(int len) { + char *s = malloc((size_t)len + 1); + if (s) { + memset(s, 'x', (size_t)len); + s[len] = '\0'; + } + return s; +} + +/* Regression: sections whose combined size exceeds the fixed render buffer used + * to overflow it. adr_render_section advanced pos by snprintf's return value + * (the length it WOULD have written); once pos passed buf_sz the next section + * computed a wrapped (huge) remaining size from (buf_sz - pos) and wrote past + * the stack buffer (ASan: stack-buffer-overflow WRITE). + * + * The render buffer is ST_MAX_DEGREE*ST_GROWTH = 16384. Sizes below drive the + * cursor to exactly buf_sz+8 after section "B" (a truncating write that stays + * in bounds), so section "C" performs the first out-of-bounds write right at + * the buffer's tail — into ASan's redzone, where it is reliably caught. + * Non-canonical keys render in alphabetical order (A, B, C). */ +TEST(adr_render_oversized_sections_no_overflow) { + enum { ADR_RENDER_BUFSZ = 16384 }; + cbm_adr_sections_t sec = {.count = 3}; + /* A is first: header "## A\n" (5) + value -> pos = 16000. */ + sec.keys[0] = strdup("A"); + sec.values[0] = adr_test_fill(15995); + /* B: header "\n\n## B\n" (7) + value -> intended pos = 16000+7+385 = 16392 + * (= buf_sz + 8); the write itself truncates safely in bounds. */ + sec.keys[1] = strdup("B"); + sec.values[1] = adr_test_fill(385); + /* C: rendered with pos = 16392 > buf_sz -> "\n\n" written at buf+16392. */ + sec.keys[2] = strdup("C"); + sec.values[2] = strdup("z"); + for (int i = 0; i < 3; i++) { + ASSERT_NOT_NULL(sec.values[i]); + } + char *rendered = cbm_adr_render(&sec); + /* Must not crash; result must be NUL-terminated within the render buffer. */ + ASSERT_NOT_NULL(rendered); + ASSERT_TRUE(strlen(rendered) < ADR_RENDER_BUFSZ); + free(rendered); + cbm_adr_sections_free(&sec); + PASS(); +} + TEST(adr_parse_render_roundtrip) { const char *original = "## PURPOSE\nTest project\n\n## STACK\n- Go: speed\n- SQLite: embedded\n\n" @@ -1393,6 +1439,7 @@ SUITE(store_arch) { RUN_TEST(adr_render_all_sections); RUN_TEST(adr_render_non_canonical); RUN_TEST(adr_render_empty); + RUN_TEST(adr_render_oversized_sections_no_overflow); RUN_TEST(adr_parse_render_roundtrip); RUN_TEST(adr_update_sections); RUN_TEST(adr_update_overflow);