Skip to content

Commit c8661b9

Browse files
map588map588
committed
fix(cypher,store): prevent crashes from buffer overflow, OOM, and NULL stmts
- cypher: Add bounds check in lex_string_literal to prevent stack buffer overflow on string literals >4096 bytes. Escape sequences are always parsed correctly even past the truncation boundary. - cypher: Add malloc/calloc NULL checks in parse_props, parse_rel_types, parse_in_condition, and parse_case_expr. Growth paths use non-destructive realloc (temp pointer) so accumulated elements can be freed on OOM instead of leaking through safe_realloc's free-on-failure semantics. - store: Add sqlite3_prepare_v2 return code checks at 3 sites in cbm_store_schema_info and collect_pkg_names. Partially prepared statements are finalized before returning. Schema function cleans up partially populated output on failure. collect_pkg_names returns CBM_NOT_FOUND (not 0) to distinguish errors from empty results.
1 parent 1d30971 commit c8661b9

File tree

3 files changed

+134
-25
lines changed

3 files changed

+134
-25
lines changed

‎src/cypher/cypher.c‎

Lines changed: 88 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -92,25 +92,30 @@ static void lex_string_literal(const char *input, int len, int *pos, char quote,
9292
int start = *pos;
9393
char buf[CBM_SZ_4K];
9494
int blen = 0;
95+
const int max_blen = CBM_SZ_4K - 1;
9596
while (*pos < len && input[*pos] != quote) {
9697
if (input[*pos] == '\\' && *pos + SKIP_ONE < len) {
9798
(*pos)++;
98-
switch (input[*pos]) {
99-
case 'n':
100-
buf[blen++] = '\n';
101-
break;
102-
case 't':
103-
buf[blen++] = '\t';
104-
break;
105-
case '\\':
106-
buf[blen++] = '\\';
107-
break;
108-
default:
109-
buf[blen++] = input[*pos];
110-
break;
99+
if (blen < max_blen) {
100+
switch (input[*pos]) {
101+
case 'n':
102+
buf[blen++] = '\n';
103+
break;
104+
case 't':
105+
buf[blen++] = '\t';
106+
break;
107+
case '\\':
108+
buf[blen++] = '\\';
109+
break;
110+
default:
111+
buf[blen++] = input[*pos];
112+
break;
113+
}
111114
}
112115
} else {
113-
buf[blen++] = input[*pos];
116+
if (blen < max_blen) {
117+
buf[blen++] = input[*pos];
118+
}
114119
}
115120
(*pos)++;
116121
}
@@ -469,6 +474,9 @@ static int parse_props(parser_t *p, cbm_prop_filter_t **out, int *count) {
469474
int cap = CYP_INIT_CAP4;
470475
int n = 0;
471476
cbm_prop_filter_t *arr = malloc(cap * sizeof(cbm_prop_filter_t));
477+
if (!arr) {
478+
return CBM_NOT_FOUND;
479+
}
472480

473481
while (!check(p, TOK_RBRACE) && !check(p, TOK_EOF)) {
474482
const cbm_token_t *key = expect(p, TOK_IDENT);
@@ -487,8 +495,18 @@ static int parse_props(parser_t *p, cbm_prop_filter_t **out, int *count) {
487495
}
488496

489497
if (n >= cap) {
490-
cap *= PAIR_LEN;
491-
arr = safe_realloc(arr, cap * sizeof(cbm_prop_filter_t));
498+
int new_cap = cap * PAIR_LEN;
499+
void *tmp = realloc(arr, new_cap * sizeof(cbm_prop_filter_t));
500+
if (!tmp) {
501+
for (int i = 0; i < n; i++) {
502+
free((void *)arr[i].key);
503+
free((void *)arr[i].value);
504+
}
505+
free(arr);
506+
return CBM_NOT_FOUND;
507+
}
508+
arr = tmp;
509+
cap = new_cap;
492510
}
493511
arr[n].key = heap_strdup(key->text);
494512
arr[n].value = heap_strdup(val->text);
@@ -569,6 +587,9 @@ static int parse_rel_types(parser_t *p, cbm_rel_pattern_t *out) {
569587
int cap = CYP_INIT_CAP4;
570588
int n = 0;
571589
const char **types = malloc(cap * sizeof(const char *));
590+
if (!types) {
591+
return CBM_NOT_FOUND;
592+
}
572593

573594
const cbm_token_t *t = expect(p, TOK_IDENT);
574595
if (!t) {
@@ -587,8 +608,17 @@ static int parse_rel_types(parser_t *p, cbm_rel_pattern_t *out) {
587608
return CBM_NOT_FOUND;
588609
}
589610
if (n >= cap) {
590-
cap *= PAIR_LEN;
591-
types = safe_realloc(types, cap * sizeof(const char *));
611+
int new_cap = cap * PAIR_LEN;
612+
void *tmp = realloc(types, new_cap * sizeof(const char *));
613+
if (!tmp) {
614+
for (int i = 0; i < n; i++) {
615+
free((void *)types[i]);
616+
}
617+
free(types);
618+
return CBM_NOT_FOUND;
619+
}
620+
types = (const char **)tmp;
621+
cap = new_cap;
592622
}
593623
types[n++] = heap_strdup(t->text);
594624
}
@@ -762,14 +792,32 @@ static cbm_expr_t *parse_in_list(parser_t *p, cbm_condition_t *c) {
762792
int vcap = CYP_INIT_CAP8;
763793
int vn = 0;
764794
const char **vals = malloc(vcap * sizeof(const char *));
795+
if (!vals) {
796+
free((void *)c->variable);
797+
free((void *)c->property);
798+
free((void *)c->op);
799+
return NULL;
800+
}
765801
while (!check(p, TOK_RBRACKET) && !check(p, TOK_EOF)) {
766802
if (vn > 0) {
767803
match(p, TOK_COMMA);
768804
}
769805
if (check(p, TOK_STRING) || check(p, TOK_NUMBER)) {
770806
if (vn >= vcap) {
771-
vcap *= PAIR_LEN;
772-
vals = safe_realloc(vals, vcap * sizeof(const char *));
807+
int new_vcap = vcap * PAIR_LEN;
808+
void *tmp = realloc((void *)vals, new_vcap * sizeof(const char *));
809+
if (!tmp) {
810+
for (int i = 0; i < vn; i++) {
811+
free((void *)vals[i]);
812+
}
813+
free((void *)vals);
814+
free((void *)c->variable);
815+
free((void *)c->property);
816+
free((void *)c->op);
817+
return NULL;
818+
}
819+
vals = (const char **)tmp;
820+
vcap = new_vcap;
773821
}
774822
vals[vn++] = heap_strdup(advance(p)->text);
775823
} else {
@@ -1061,8 +1109,15 @@ static const char *parse_value_literal(parser_t *p) {
10611109
static cbm_case_expr_t *parse_case_expr(parser_t *p) {
10621110
/* CASE already consumed */
10631111
cbm_case_expr_t *kase = calloc(CBM_ALLOC_ONE, sizeof(cbm_case_expr_t));
1112+
if (!kase) {
1113+
return NULL;
1114+
}
10641115
int bcap = CYP_INIT_CAP4;
10651116
kase->branches = malloc(bcap * sizeof(cbm_case_branch_t));
1117+
if (!kase->branches) {
1118+
free(kase);
1119+
return NULL;
1120+
}
10661121

10671122
while (check(p, TOK_WHEN)) {
10681123
advance(p);
@@ -1073,8 +1128,19 @@ static cbm_case_expr_t *parse_case_expr(parser_t *p) {
10731128
}
10741129
const char *then_val = parse_value_literal(p);
10751130
if (kase->branch_count >= bcap) {
1076-
bcap *= PAIR_LEN;
1077-
kase->branches = safe_realloc(kase->branches, bcap * sizeof(cbm_case_branch_t));
1131+
int new_bcap = bcap * PAIR_LEN;
1132+
void *tmp = realloc(kase->branches, new_bcap * sizeof(cbm_case_branch_t));
1133+
if (!tmp) {
1134+
expr_free(when);
1135+
for (int i = 0; i < kase->branch_count; i++) {
1136+
expr_free(kase->branches[i].when_expr);
1137+
}
1138+
free(kase->branches);
1139+
free(kase);
1140+
return NULL;
1141+
}
1142+
kase->branches = tmp;
1143+
bcap = new_bcap;
10781144
}
10791145
kase->branches[kase->branch_count++] =
10801146
(cbm_case_branch_t){.when_expr = when, .then_val = then_val};

‎src/store/store.c‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2552,7 +2552,12 @@ int cbm_store_get_schema(cbm_store_t *s, const char *project, cbm_schema_info_t
25522552
const char *sql = "SELECT label, COUNT(*) FROM nodes WHERE project = ?1 GROUP BY label "
25532553
"ORDER BY COUNT(*) DESC;";
25542554
sqlite3_stmt *stmt = NULL;
2555-
sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL);
2555+
if (sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK || !stmt) {
2556+
if (stmt) {
2557+
sqlite3_finalize(stmt);
2558+
}
2559+
return CBM_NOT_FOUND;
2560+
}
25562561
bind_text(stmt, SKIP_ONE, project);
25572562

25582563
int cap = ST_INIT_CAP_8;
@@ -2577,7 +2582,13 @@ int cbm_store_get_schema(cbm_store_t *s, const char *project, cbm_schema_info_t
25772582
const char *sql = "SELECT type, COUNT(*) FROM edges WHERE project = ?1 GROUP BY type ORDER "
25782583
"BY COUNT(*) DESC;";
25792584
sqlite3_stmt *stmt = NULL;
2580-
sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL);
2585+
if (sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK || !stmt) {
2586+
if (stmt) {
2587+
sqlite3_finalize(stmt);
2588+
}
2589+
cbm_store_schema_free(out);
2590+
return CBM_NOT_FOUND;
2591+
}
25812592
bind_text(stmt, SKIP_ONE, project);
25822593

25832594
int cap = ST_INIT_CAP_8;
@@ -3283,7 +3294,12 @@ static bool pkg_in_list(const char *pkg, char **list, int count) {
32833294
static int collect_pkg_names(cbm_store_t *s, const char *sql, const char *project, char **pkgs,
32843295
int max_pkgs) {
32853296
sqlite3_stmt *stmt = NULL;
3286-
sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL);
3297+
if (sqlite3_prepare_v2(s->db, sql, CBM_NOT_FOUND, &stmt, NULL) != SQLITE_OK || !stmt) {
3298+
if (stmt) {
3299+
sqlite3_finalize(stmt);
3300+
}
3301+
return CBM_NOT_FOUND;
3302+
}
32873303
bind_text(stmt, SKIP_ONE, project);
32883304
int count = 0;
32893305
while (sqlite3_step(stmt) == SQLITE_ROW && count < max_pkgs) {

‎tests/test_cypher.c‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,32 @@ TEST(cypher_lex_single_quote_string) {
7878
PASS();
7979
}
8080

81+
TEST(cypher_lex_string_overflow) {
82+
/* Build a string literal longer than 4096 bytes to verify we don't
83+
* overflow the stack buffer in lex_string_literal. */
84+
const int big = 5000;
85+
/* query: "AAAA...A" (quotes included) */
86+
char *query = malloc(big + 3); /* quote + big chars + quote + NUL */
87+
ASSERT_NOT_NULL(query);
88+
query[0] = '"';
89+
memset(query + 1, 'A', big);
90+
query[big + 1] = '"';
91+
query[big + 2] = '\0';
92+
93+
cbm_lex_result_t r = {0};
94+
int rc = cbm_lex(query, &r);
95+
ASSERT_EQ(rc, 0);
96+
ASSERT_NULL(r.error);
97+
ASSERT_GTE(r.count, 1);
98+
ASSERT_EQ(r.tokens[0].type, TOK_STRING);
99+
/* The string should be truncated to CBM_SZ_4K - 1 (4095) characters. */
100+
ASSERT_EQ((int)strlen(r.tokens[0].text), 4095);
101+
102+
cbm_lex_free(&r);
103+
free(query);
104+
PASS();
105+
}
106+
81107
TEST(cypher_lex_number) {
82108
cbm_lex_result_t r = {0};
83109
int rc = cbm_lex("42 3.14", &r);
@@ -2064,6 +2090,7 @@ SUITE(cypher) {
20642090
RUN_TEST(cypher_lex_relationship);
20652091
RUN_TEST(cypher_lex_string_literal);
20662092
RUN_TEST(cypher_lex_single_quote_string);
2093+
RUN_TEST(cypher_lex_string_overflow);
20672094
RUN_TEST(cypher_lex_number);
20682095
RUN_TEST(cypher_lex_operators);
20692096
RUN_TEST(cypher_lex_keywords_case_insensitive);

0 commit comments

Comments
 (0)