Skip to content

Commit 74d15a6

Browse files
rainderclaude
andcommitted
fix(pipeline): prevent stack-buffer-overflow in append_args_json
A call carrying enough long arguments drove append_args_json()'s running position past the fixed CBM_SZ_2K `props` stack buffer in emit_normal_calls_edge(): format_call_arg() returns snprintf's *untruncated* length, so `pos += (size_t)n` could exceed `bufsize`, after which the trailing `buf[pos] = '\0'` (and `buf[pos++] = ']'`) wrote out of bounds. The stack canary caught it as SIGABRT, so full-repo indexing of large TypeScript codebases crashed the server in the parallel resolve pass (emit_service_edge -> emit_normal_calls_edge -> finalize_and_emit -> append_args_json). Confirmed with AddressSanitizer: stack-buffer-overflow WRITE at pass_parallel.c:1124, 'props' (2048 B). Fix: when an argument does not fully fit, roll back to before its separator and stop appending (atomic field, matching append_json_string's behaviour), so `pos` can never advance past the buffer. Add regression test parallel_args_json_no_overflow: indexes a fixture whose single call carries 60 long string args (args JSON well past 2 KB); under the ASan test build it aborts without this fix and passes with it. Signed-off-by: Andrius Skerla <1492322+rainder@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e599df1 commit 74d15a6

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

src/pipeline/pass_parallel.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,15 +1108,22 @@ static size_t append_args_json(char *buf, size_t bufsize, size_t pos, const CBMC
11081108
pos += (size_t)n;
11091109
for (int i = 0; i < call->arg_count && pos < bufsize - CBM_ARG_JSON_GUARD; i++) {
11101110
const CBMCallArg *a = &call->args[i];
1111+
size_t mark = pos; /* rollback point (before the separator) */
11111112
if (i > 0 && pos < bufsize - SKIP_ONE) {
11121113
buf[pos++] = ',';
11131114
}
11141115
char expr_buf[CBM_SZ_128];
11151116
sanitize_expr(expr_buf, a->expr);
11161117
n = format_call_arg(buf + pos, bufsize - pos, a, expr_buf);
1117-
if (n > 0) {
1118-
pos += (size_t)n;
1118+
/* snprintf returns the UNtruncated length: if the arg did not fully
1119+
* fit, advancing pos by n would push it past buf and the buf[pos]
1120+
* writes below would overflow. Drop the arg whole (atomic field —
1121+
* keeps the array valid) and stop appending. */
1122+
if (n <= 0 || (size_t)n >= bufsize - pos) {
1123+
pos = mark;
1124+
break;
11191125
}
1126+
pos += (size_t)n;
11201127
}
11211128
if (pos < bufsize - SKIP_ONE) {
11221129
buf[pos++] = ']';

tests/test_parallel.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,54 @@ TEST(parallel_empty_files) {
341341
PASS();
342342
}
343343

344+
/* ── Regression: args JSON must not overflow the props buffer ──────── */
345+
346+
/* A call with many long string arguments makes append_args_json()'s running
347+
* position exceed the fixed CBM_SZ_2K `props` stack buffer in
348+
* emit_normal_calls_edge(): format_call_arg() returns snprintf's UNtruncated
349+
* length, so pos += n could run past the buffer and the trailing
350+
* buf[pos]='\0' wrote out of bounds (stack-buffer-overflow; caught by the
351+
* stack canary as a SIGABRT on real repos). This indexes a fixture whose
352+
* single call carries enough long args to drive pos past 2 KB; under the
353+
* ASan test build a regression aborts here. */
354+
TEST(parallel_args_json_no_overflow) {
355+
char dir[256];
356+
snprintf(dir, sizeof(dir), "/tmp/cbm_argov_XXXXXX");
357+
ASSERT_TRUE(cbm_mkdtemp(dir) != NULL);
358+
359+
char path[512];
360+
snprintf(path, sizeof(path), "%s/app.ts", dir);
361+
FILE *f = fopen(path, "w");
362+
ASSERT_TRUE(f != NULL);
363+
fputs("function sink(...xs: string[]) { return xs; }\n", f);
364+
fputs("function caller() {\n sink(\n", f);
365+
for (int i = 0; i < 60; i++) {
366+
/* 100-char string literal per arg; 60 args => args JSON well past the
367+
* 2 KB props buffer, forcing the pre-fix overshoot. */
368+
fputs(" \"", f);
369+
for (int j = 0; j < 100; j++)
370+
fputc('a' + (i % 26), f);
371+
fputs(i < 59 ? "\",\n" : "\"\n", f);
372+
}
373+
fputs(" );\n}\n", f);
374+
fclose(f);
375+
376+
cbm_discover_opts_t opts = {.mode = CBM_MODE_FULL};
377+
cbm_file_info_t *files = NULL;
378+
int file_count = 0;
379+
ASSERT_EQ(cbm_discover(dir, &opts, &files, &file_count), 0);
380+
ASSERT_GT(file_count, 0);
381+
382+
cbm_gbuf_t *gbuf = run_parallel("argov-test", dir, files, file_count, 4);
383+
ASSERT_TRUE(gbuf != NULL);
384+
ASSERT_GT(cbm_gbuf_edge_count(gbuf), 0);
385+
386+
cbm_gbuf_free(gbuf);
387+
cbm_discover_free(files, file_count);
388+
th_rmtree(dir);
389+
PASS();
390+
}
391+
344392
/* ── Graph buffer merge tests ─────────────────────────────────────── */
345393

346394
TEST(gbuf_shared_ids_unique) {
@@ -680,6 +728,7 @@ SUITE(parallel) {
680728
RUN_TEST(parallel_implements_parity);
681729
RUN_TEST(parallel_total_edges);
682730
RUN_TEST(parallel_empty_files);
731+
RUN_TEST(parallel_args_json_no_overflow);
683732

684733
/* Cleanup shared state */
685734
parity_teardown();

0 commit comments

Comments
 (0)