diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 98ffb4c97..d06ff9c1e 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -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)) { diff --git a/tests/test_cypher.c b/tests/test_cypher.c index fc3a06932..fff14e110 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -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);