Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/cypher/cypher.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
47 changes: 47 additions & 0 deletions tests/test_cypher.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */

Expand Down Expand Up @@ -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);
Expand Down
Loading