fix(cypher): bound UNWIND literal-list assembly buffer to prevent stack overflow

Signed-off-by: SEPURI-SAI-KRISHNA <saik20533@gmail.com>
This commit is contained in:
SEPURI-SAI-KRISHNA
2026-07-19 11:57:38 +05:30
parent 7d6cdb23ef
commit d0457f2ba9
2 changed files with 61 additions and 4 deletions
+14 -4
View File
@@ -1836,23 +1836,33 @@ static void parse_unwind_clause(parser_t *p, cbm_query_t *q) {
advance(p);
char buf[CBM_SZ_2K] = "[";
int blen = SKIP_ONE;
/* snprintf returns the length it WOULD have written, so a single
* oversized token (string literals lex up to CBM_SZ_4K-1) can push
* blen past sizeof(buf). Clamp after every write and guard the raw
* buf[blen++] stores, mirroring format_collect_list(). */
const int cap = (int)sizeof(buf);
while (!check(p, TOK_RBRACKET) && !check(p, TOK_EOF)) {
if (blen > SKIP_ONE) {
if (blen > SKIP_ONE && blen < cap - SKIP_ONE) {
buf[blen++] = ',';
}
if (check(p, TOK_STRING)) {
blen += snprintf(buf + blen, sizeof(buf) - blen, "\"%s\"", peek(p)->text);
blen += snprintf(buf + blen, (size_t)(cap - blen), "\"%s\"", peek(p)->text);
advance(p);
} else if (check(p, TOK_NUMBER)) {
blen += snprintf(buf + blen, sizeof(buf) - blen, "%s", peek(p)->text);
blen += snprintf(buf + blen, (size_t)(cap - blen), "%s", peek(p)->text);
advance(p);
} else {
advance(p);
}
if (blen >= cap) {
blen = cap - SKIP_ONE;
}
match(p, TOK_COMMA);
}
expect(p, TOK_RBRACKET);
buf[blen++] = ']';
if (blen < cap - SKIP_ONE) {
buf[blen++] = ']';
}
buf[blen] = '\0';
q->unwind_expr = heap_strdup(buf);
} else if (check(p, TOK_IDENT)) {
+47
View File
@@ -2762,6 +2762,51 @@ TEST(cypher_parse_unwind_var) {
PASS();
}
/* Regression: an UNWIND literal list whose element is longer than the 2KB
* assembly buffer used to overflow the stack. snprintf reports the length it
* WOULD have written, so blen ran past sizeof(buf) and the trailing
* buf[blen++]=']' / buf[blen]='\0' wrote out of bounds (ASan: stack-buffer-
* overflow). The query text is agent-controlled via the MCP query tool. */
TEST(cypher_parse_unwind_oversized_literal_no_overflow) {
char query[4096];
char big[3000];
memset(big, 'a', sizeof(big) - 1);
big[sizeof(big) - 1] = '\0';
snprintf(query, sizeof(query), "UNWIND [\"%s\"] AS x MATCH (f) RETURN f.name", big);
cbm_query_t *q = NULL;
char *err = NULL;
int rc = cbm_cypher_parse(query, &q, &err);
/* Must not crash and must produce a NUL-terminated, in-bounds expression. */
ASSERT_EQ(rc, 0);
ASSERT_NOT_NULL(q->unwind_expr);
ASSERT_STR_EQ(q->unwind_alias, "x");
cbm_query_free(q);
PASS();
}
/* Regression: many oversized elements accumulate blen well past the buffer,
* which also underflowed the (size_t)(cap - blen) length passed to snprintf. */
TEST(cypher_parse_unwind_many_elements_no_overflow) {
/* 200 elements (~20 chars each) accumulate well past the 2KB assembly
* buffer, which also underflowed the (size_t)(cap - blen) length. */
char query[8192];
int off = snprintf(query, sizeof(query), "UNWIND [");
for (int i = 0; i < 200; i++) {
off += snprintf(query + off, sizeof(query) - (size_t)off, "%s\"element_value_%d\"",
i ? "," : "", i);
}
snprintf(query + off, sizeof(query) - (size_t)off, "] AS x MATCH (f) RETURN f.name");
cbm_query_t *q = NULL;
char *err = NULL;
int rc = cbm_cypher_parse(query, &q, &err);
ASSERT_EQ(rc, 0);
ASSERT_NOT_NULL(q->unwind_expr);
cbm_query_free(q);
PASS();
}
/* ── Issue #389 group: Cypher feature reproductions ─────────────────
* Each asserts the CORRECT behavior; a failure reproduces the bug. */
@@ -3218,6 +3263,8 @@ SUITE(cypher) {
/* Phase 9: UNWIND */
RUN_TEST(cypher_parse_unwind);
RUN_TEST(cypher_parse_unwind_var);
RUN_TEST(cypher_parse_unwind_oversized_literal_no_overflow);
RUN_TEST(cypher_parse_unwind_many_elements_no_overflow);
RUN_TEST(cypher_wide_return_projection_bounded);
/* Composite property projection (arrays/objects, escaped quotes) */
RUN_TEST(cypher_exec_prop_array_with_internal_commas);