Skip to content

Commit d0457f2

Browse files
fix(cypher): bound UNWIND literal-list assembly buffer to prevent stack overflow
Signed-off-by: SEPURI-SAI-KRISHNA <saik20533@gmail.com>
1 parent 7d6cdb2 commit d0457f2

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

src/cypher/cypher.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,23 +1836,33 @@ static void parse_unwind_clause(parser_t *p, cbm_query_t *q) {
18361836
advance(p);
18371837
char buf[CBM_SZ_2K] = "[";
18381838
int blen = SKIP_ONE;
1839+
/* snprintf returns the length it WOULD have written, so a single
1840+
* oversized token (string literals lex up to CBM_SZ_4K-1) can push
1841+
* blen past sizeof(buf). Clamp after every write and guard the raw
1842+
* buf[blen++] stores, mirroring format_collect_list(). */
1843+
const int cap = (int)sizeof(buf);
18391844
while (!check(p, TOK_RBRACKET) && !check(p, TOK_EOF)) {
1840-
if (blen > SKIP_ONE) {
1845+
if (blen > SKIP_ONE && blen < cap - SKIP_ONE) {
18411846
buf[blen++] = ',';
18421847
}
18431848
if (check(p, TOK_STRING)) {
1844-
blen += snprintf(buf + blen, sizeof(buf) - blen, "\"%s\"", peek(p)->text);
1849+
blen += snprintf(buf + blen, (size_t)(cap - blen), "\"%s\"", peek(p)->text);
18451850
advance(p);
18461851
} else if (check(p, TOK_NUMBER)) {
1847-
blen += snprintf(buf + blen, sizeof(buf) - blen, "%s", peek(p)->text);
1852+
blen += snprintf(buf + blen, (size_t)(cap - blen), "%s", peek(p)->text);
18481853
advance(p);
18491854
} else {
18501855
advance(p);
18511856
}
1857+
if (blen >= cap) {
1858+
blen = cap - SKIP_ONE;
1859+
}
18521860
match(p, TOK_COMMA);
18531861
}
18541862
expect(p, TOK_RBRACKET);
1855-
buf[blen++] = ']';
1863+
if (blen < cap - SKIP_ONE) {
1864+
buf[blen++] = ']';
1865+
}
18561866
buf[blen] = '\0';
18571867
q->unwind_expr = heap_strdup(buf);
18581868
} else if (check(p, TOK_IDENT)) {

tests/test_cypher.c

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2762,6 +2762,51 @@ TEST(cypher_parse_unwind_var) {
27622762
PASS();
27632763
}
27642764

2765+
/* Regression: an UNWIND literal list whose element is longer than the 2KB
2766+
* assembly buffer used to overflow the stack. snprintf reports the length it
2767+
* WOULD have written, so blen ran past sizeof(buf) and the trailing
2768+
* buf[blen++]=']' / buf[blen]='\0' wrote out of bounds (ASan: stack-buffer-
2769+
* overflow). The query text is agent-controlled via the MCP query tool. */
2770+
TEST(cypher_parse_unwind_oversized_literal_no_overflow) {
2771+
char query[4096];
2772+
char big[3000];
2773+
memset(big, 'a', sizeof(big) - 1);
2774+
big[sizeof(big) - 1] = '\0';
2775+
snprintf(query, sizeof(query), "UNWIND [\"%s\"] AS x MATCH (f) RETURN f.name", big);
2776+
2777+
cbm_query_t *q = NULL;
2778+
char *err = NULL;
2779+
int rc = cbm_cypher_parse(query, &q, &err);
2780+
/* Must not crash and must produce a NUL-terminated, in-bounds expression. */
2781+
ASSERT_EQ(rc, 0);
2782+
ASSERT_NOT_NULL(q->unwind_expr);
2783+
ASSERT_STR_EQ(q->unwind_alias, "x");
2784+
cbm_query_free(q);
2785+
PASS();
2786+
}
2787+
2788+
/* Regression: many oversized elements accumulate blen well past the buffer,
2789+
* which also underflowed the (size_t)(cap - blen) length passed to snprintf. */
2790+
TEST(cypher_parse_unwind_many_elements_no_overflow) {
2791+
/* 200 elements (~20 chars each) accumulate well past the 2KB assembly
2792+
* buffer, which also underflowed the (size_t)(cap - blen) length. */
2793+
char query[8192];
2794+
int off = snprintf(query, sizeof(query), "UNWIND [");
2795+
for (int i = 0; i < 200; i++) {
2796+
off += snprintf(query + off, sizeof(query) - (size_t)off, "%s\"element_value_%d\"",
2797+
i ? "," : "", i);
2798+
}
2799+
snprintf(query + off, sizeof(query) - (size_t)off, "] AS x MATCH (f) RETURN f.name");
2800+
2801+
cbm_query_t *q = NULL;
2802+
char *err = NULL;
2803+
int rc = cbm_cypher_parse(query, &q, &err);
2804+
ASSERT_EQ(rc, 0);
2805+
ASSERT_NOT_NULL(q->unwind_expr);
2806+
cbm_query_free(q);
2807+
PASS();
2808+
}
2809+
27652810
/* ── Issue #389 group: Cypher feature reproductions ─────────────────
27662811
* Each asserts the CORRECT behavior; a failure reproduces the bug. */
27672812

@@ -3218,6 +3263,8 @@ SUITE(cypher) {
32183263
/* Phase 9: UNWIND */
32193264
RUN_TEST(cypher_parse_unwind);
32203265
RUN_TEST(cypher_parse_unwind_var);
3266+
RUN_TEST(cypher_parse_unwind_oversized_literal_no_overflow);
3267+
RUN_TEST(cypher_parse_unwind_many_elements_no_overflow);
32213268
RUN_TEST(cypher_wide_return_projection_bounded);
32223269
/* Composite property projection (arrays/objects, escaped quotes) */
32233270
RUN_TEST(cypher_exec_prop_array_with_internal_commas);

0 commit comments

Comments
 (0)