mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix: bound response buffers, parse depth and glob matching
A pass over places where a bound was either miscalculated or missing.
UI response builders (src/ui/http_server.c) mixed the clamping helper
http_appendf with raw cursor indexing. Route the per-line separators and
quotes through the helper too, so the cursor cannot leave the buffer once
output saturates it, and terminate explicitly afterwards — the helper pins
the cursor and then writes nothing, which would otherwise leave the "%s"
reply with no terminator in range. The log buffer was budgeted at
LOG_LINE_MAX + 10 per line while JSON escaping can double every byte, so
size it for the escaped worst case. Job status entries now escape their path
and error fields, which were interpolated raw.
Cypher (src/cypher/cypher.c): the WHERE grammar descends once per nested '('
and once per NOT, so parse depth followed the query text rather than
anything bounded. Track depth on parser_t and refuse past 256 levels with a
parse error. Bounding parse depth also bounds the resulting tree, so the
recursive evaluator inherits the limit.
Gitignore (src/discover/gitignore.c): '**' retries the remainder at every
position and consecutive groups multiply, so match cost is exponential in
the number of groups. Thread a step budget through the matcher and give up
past 20000 steps, reporting no-match so a pathological pattern fails to
ignore rather than ignoring the wrong files. glob_match keeps its name so
recursion_whitelist.h still describes the functions that recurse; the new
non-recursive wrapper that seeds the budget is glob_match_bounded.
Call extraction (internal/cbm/extract_calls.c): extract_fp_callee recursed
once per applied argument, so stack use followed the parse-tree depth of the
indexed file. Rewritten as a left-spine loop. Reassigning node/nk before
continuing leaves the fall-through cases where the recursive form left them,
so behaviour is unchanged and the extraction suite is untouched.
CALLS props (src/pipeline/pass_calls.c): `cap - pos - PAIR_LEN` wraps once
pos reaches cap - PAIR_LEN and stops bounding the copy below. Guard
additively, matching the closing write in the same function.
Shell-bound paths (src/foundation/str_util.*): the three git shell-out sites
each wrap a repo path in cmd.exe-compatible double quotes, where %VAR%,
!VAR! and ^ remain active. Two carried private copies of the check that
rejects those three on Windows and the third used the bare validator.
Promote the stricter form to cbm_validate_shell_path_arg and route all three
through it, so the copies cannot drift apart again.
Tests cover the buffer bounds, the parse-depth refusal, matcher termination
and the validator. The bounds tests run in a forked child so a violation
surfaces as a signal instead of silently.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
4bbe0984f7
commit
886a8856a4
14 files changed
+475
-69
No files matched your search
@@ -326,26 +326,36 @@ static char *extract_callee_from_fields(CBMArena *a, TSNode node, const char *so
|
||||
}
|
||||
|
||||
// Haskell/OCaml: extract callee from apply/infix nodes.
|
||||
/* Apply-style node kinds whose function head can nest another application. */
|
||||
static bool fp_is_apply_kind(const char *kind) {
|
||||
return strcmp(kind, "apply") == 0 || strcmp(kind, "application_expression") == 0 ||
|
||||
strcmp(kind, "exp_apply") == 0;
|
||||
}
|
||||
|
||||
static char *extract_fp_callee(CBMArena *a, TSNode node, const char *source, const char *nk) {
|
||||
if (strcmp(nk, "apply") == 0 || strcmp(nk, "application_expression") == 0 ||
|
||||
strcmp(nk, "exp_apply") == 0) {
|
||||
if (ts_node_child_count(node) > 0) {
|
||||
TSNode callee = ts_node_child(node, 0);
|
||||
const char *ck = ts_node_type(callee);
|
||||
if (strcmp(ck, "identifier") == 0 || strcmp(ck, "variable") == 0 ||
|
||||
strcmp(ck, "constructor") == 0 || strcmp(ck, "value_path") == 0 ||
|
||||
/* PureScript: exp_apply's function head is an `exp_name` whose
|
||||
* text is the (possibly qualified) function name. */
|
||||
strcmp(ck, "exp_name") == 0) {
|
||||
return cbm_node_text(a, callee, source);
|
||||
}
|
||||
/* Curried application `f a b` nests exp_apply/apply — descend the
|
||||
* function head to recover the leftmost callee. */
|
||||
if (strcmp(ck, "exp_apply") == 0 || strcmp(ck, "apply") == 0 ||
|
||||
strcmp(ck, "application_expression") == 0) {
|
||||
return extract_fp_callee(a, callee, source, ck);
|
||||
}
|
||||
/* Curried application `f a b …` nests one apply node per argument on the
|
||||
* function head. Walk that left spine iteratively: recursing once per
|
||||
* application makes stack use follow the parse-tree depth of the indexed
|
||||
* file, and a long enough chain runs out. The other descendant finders in
|
||||
* this tree are already depth-bounded.
|
||||
*
|
||||
* Reassigning node/nk before continuing leaves the fall-through below
|
||||
* operating on the innermost apply node — where the recursive form left it. */
|
||||
while (fp_is_apply_kind(nk) && ts_node_child_count(node) > 0) {
|
||||
TSNode callee = ts_node_child(node, 0);
|
||||
const char *ck = ts_node_type(callee);
|
||||
if (strcmp(ck, "identifier") == 0 || strcmp(ck, "variable") == 0 ||
|
||||
strcmp(ck, "constructor") == 0 || strcmp(ck, "value_path") == 0 ||
|
||||
/* PureScript: exp_apply's function head is an `exp_name` whose
|
||||
* text is the (possibly qualified) function name. */
|
||||
strcmp(ck, "exp_name") == 0) {
|
||||
return cbm_node_text(a, callee, source);
|
||||
}
|
||||
if (!fp_is_apply_kind(ck)) {
|
||||
break;
|
||||
}
|
||||
node = callee;
|
||||
nk = ck;
|
||||
}
|
||||
if (strcmp(nk, "infix") == 0 || strcmp(nk, "infix_expression") == 0) {
|
||||
TSNode op = ts_node_child_by_field_name(node, TS_FIELD("operator"));
|
||||
|
||||
@@ -432,13 +432,36 @@ void cbm_lex_free(cbm_lex_result_t *r) {
|
||||
* PARSER
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
|
||||
/* The WHERE grammar descends once per nested '(' and once per NOT, so parse
|
||||
* depth follows the query text rather than anything bounded. A few tens of KB of
|
||||
* either exhausts the stack before any semantic limit applies. 256 is far past
|
||||
* any hand-written or generated query while leaving the stack untouched. */
|
||||
enum { CYPHER_MAX_PARSE_DEPTH = 256 };
|
||||
|
||||
typedef struct {
|
||||
const cbm_token_t *tokens;
|
||||
int count;
|
||||
int pos;
|
||||
int depth; /* current recursive-descent depth; see CYPHER_MAX_PARSE_DEPTH */
|
||||
char error[CBM_SZ_512];
|
||||
} parser_t;
|
||||
|
||||
/* Enter one level of recursive descent. Returns false when the cap is hit, in
|
||||
* which case the caller must return NULL without recursing. */
|
||||
static bool parse_depth_enter(parser_t *p) {
|
||||
if (p->depth >= CYPHER_MAX_PARSE_DEPTH) {
|
||||
snprintf(p->error, sizeof(p->error), "expression nested deeper than %d levels",
|
||||
CYPHER_MAX_PARSE_DEPTH);
|
||||
return false;
|
||||
}
|
||||
p->depth++;
|
||||
return true;
|
||||
}
|
||||
|
||||
static void parse_depth_leave(parser_t *p) {
|
||||
p->depth--;
|
||||
}
|
||||
|
||||
static const cbm_token_t *peek(parser_t *p) {
|
||||
if (p->pos >= p->count) {
|
||||
return &p->tokens[p->count - SKIP_ONE]; /* EOF */
|
||||
@@ -1203,7 +1226,11 @@ static cbm_expr_t *parse_condition_expr(parser_t *p) {
|
||||
/* Atom: ( expr ) | condition */
|
||||
static cbm_expr_t *parse_atom_expr(parser_t *p) { // NOLINT(misc-no-recursion)
|
||||
if (match(p, TOK_LPAREN)) {
|
||||
if (!parse_depth_enter(p)) {
|
||||
return NULL;
|
||||
}
|
||||
cbm_expr_t *e = parse_or_expr(p);
|
||||
parse_depth_leave(p);
|
||||
expect(p, TOK_RPAREN);
|
||||
return e;
|
||||
}
|
||||
@@ -1213,7 +1240,11 @@ static cbm_expr_t *parse_atom_expr(parser_t *p) { // NOLINT(misc-no-recursion)
|
||||
/* NOT: NOT atom | atom */
|
||||
static cbm_expr_t *parse_not_expr(parser_t *p) { // NOLINT(misc-no-recursion)
|
||||
if (match(p, TOK_NOT)) {
|
||||
if (!parse_depth_enter(p)) {
|
||||
return NULL;
|
||||
}
|
||||
cbm_expr_t *child = parse_not_expr(p);
|
||||
parse_depth_leave(p);
|
||||
return child ? expr_not(child) : NULL;
|
||||
}
|
||||
return parse_atom_expr(p);
|
||||
|
||||
+44
-17
@@ -39,17 +39,31 @@ struct cbm_gitignore {
|
||||
|
||||
/* ── Pattern matching engine ─────────────────────────────────────── */
|
||||
|
||||
/* Backtracking budget. `**` retries the remainder at every position, and
|
||||
* consecutive literal + `**` groups multiply, so cost is exponential in the
|
||||
* number of groups. Patterns come from a committed .gitignore / .cbmignore and
|
||||
* every discovered path is matched against every pattern, so a small ignore file
|
||||
* can make discovery take unbounded time. Cap the match steps per
|
||||
* (pattern, path) pair.
|
||||
*
|
||||
* Exhausting the budget reports "no match", so a pathological pattern fails to
|
||||
* ignore rather than ignoring the wrong files: the file gets indexed, which is
|
||||
* the recoverable direction. The cap is far above any real pattern — ordinary
|
||||
* globs finish in tens of steps. */
|
||||
enum { GI_MATCH_MAX_STEPS = 20000 };
|
||||
|
||||
/* Forward declaration for recursive calls. */
|
||||
static bool glob_match(const char *pat, const char *str); // NOLINT(misc-no-recursion)
|
||||
static bool glob_match(const char *pat, const char *str,
|
||||
int *budget); // NOLINT(misc-no-recursion)
|
||||
|
||||
/* Match a ** (doublestar-slash) pattern: try rest at every / boundary. */
|
||||
static bool glob_match_doublestar_slash(const char *pat, // NOLINT(misc-no-recursion)
|
||||
const char *str) {
|
||||
if (glob_match(pat, str)) {
|
||||
const char *str, int *budget) {
|
||||
if (glob_match(pat, str, budget)) {
|
||||
return true;
|
||||
}
|
||||
for (const char *s = str; *s; s++) {
|
||||
if (*s == '/' && glob_match(pat, s + SKIP_ONE)) {
|
||||
if (*s == '/' && glob_match(pat, s + SKIP_ONE, budget)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -58,9 +72,9 @@ static bool glob_match_doublestar_slash(const char *pat, // NOLINT(misc-no-recur
|
||||
|
||||
/* Match a ** (doublestar) followed by non-slash: try at every position. */
|
||||
static bool glob_match_doublestar_any(const char *pat, // NOLINT(misc-no-recursion)
|
||||
const char *str) {
|
||||
const char *str, int *budget) {
|
||||
for (const char *s = str;; s++) {
|
||||
if (glob_match(pat, s)) {
|
||||
if (glob_match(pat, s, budget)) {
|
||||
return true;
|
||||
}
|
||||
if (!*s) {
|
||||
@@ -70,9 +84,10 @@ static bool glob_match_doublestar_any(const char *pat, // NOLINT(misc-no-recursi
|
||||
}
|
||||
|
||||
/* Match a * (single star): match any sequence not containing /. */
|
||||
static bool glob_match_star(const char *pat, const char *str) { // NOLINT(misc-no-recursion)
|
||||
static bool glob_match_star(const char *pat, const char *str,
|
||||
int *budget) { // NOLINT(misc-no-recursion)
|
||||
for (const char *s = str;; s++) {
|
||||
if (glob_match(pat, s)) {
|
||||
if (glob_match(pat, s, budget)) {
|
||||
return true;
|
||||
}
|
||||
if (!*s || *s == '/') {
|
||||
@@ -119,24 +134,29 @@ static bool glob_match_charclass(const char *pat, char ch, const char **pat_out)
|
||||
* Handles: * (non-slash), ** (any path), ? (single non-slash), [class]
|
||||
*/
|
||||
/* Handle ** at current position. Returns match result. */
|
||||
static bool glob_match_doublestar(const char *pat, const char *str) { // NOLINT(misc-no-recursion)
|
||||
static bool glob_match_doublestar(const char *pat, const char *str,
|
||||
int *budget) { // NOLINT(misc-no-recursion)
|
||||
if (pat[GI_CHAR_IDX2] == '/') {
|
||||
return glob_match_doublestar_slash(pat + GI_SKIP3, str);
|
||||
return glob_match_doublestar_slash(pat + GI_SKIP3, str, budget);
|
||||
}
|
||||
if (pat[GI_CHAR_IDX2] == '\0') {
|
||||
return true;
|
||||
}
|
||||
return glob_match_doublestar_any(pat + GI_CHAR_IDX2, str);
|
||||
return glob_match_doublestar_any(pat + GI_CHAR_IDX2, str, budget);
|
||||
}
|
||||
|
||||
static bool glob_match(const char *pat, const char *str) { // NOLINT(misc-no-recursion)
|
||||
static bool glob_match(const char *pat, const char *str,
|
||||
int *budget) { // NOLINT(misc-no-recursion)
|
||||
if (--*budget <= 0) {
|
||||
return false; /* budget exhausted — see GI_MATCH_MAX_STEPS */
|
||||
}
|
||||
while (*pat && *str) {
|
||||
if (pat[0] == '*' && pat[GI_CHAR_IDX1] == '*') {
|
||||
return glob_match_doublestar(pat, str);
|
||||
return glob_match_doublestar(pat, str, budget);
|
||||
}
|
||||
|
||||
if (*pat == '*') {
|
||||
return glob_match_star(pat + SKIP_ONE, str);
|
||||
return glob_match_star(pat + SKIP_ONE, str, budget);
|
||||
}
|
||||
|
||||
if (*pat == '?') {
|
||||
@@ -171,6 +191,13 @@ static bool glob_match(const char *pat, const char *str) { // NOLINT(misc-no-rec
|
||||
return *pat == '\0' && *str == '\0';
|
||||
}
|
||||
|
||||
/* Match one pattern against one path with a fresh backtracking budget. Not
|
||||
* recursive itself — it seeds the budget and hands off to the engine. */
|
||||
static bool glob_match_bounded(const char *pat, const char *str) {
|
||||
int budget = GI_MATCH_MAX_STEPS;
|
||||
return glob_match(pat, str, &budget);
|
||||
}
|
||||
|
||||
/* ── Pattern parsing ─────────────────────────────────────────────── */
|
||||
|
||||
static void gi_add_pattern(cbm_gitignore_t *gi, const char *line, int len) {
|
||||
@@ -320,7 +347,7 @@ cbm_gitignore_t *cbm_gitignore_load(const char *path) {
|
||||
|
||||
/* Match a non-rooted pattern against basename and path suffixes. */
|
||||
static bool match_unrooted(const char *pattern, const char *rel_path, const char *basename) {
|
||||
if (glob_match(pattern, basename)) {
|
||||
if (glob_match_bounded(pattern, basename)) {
|
||||
return true;
|
||||
}
|
||||
if (!strchr(rel_path, '/')) {
|
||||
@@ -329,7 +356,7 @@ static bool match_unrooted(const char *pattern, const char *rel_path, const char
|
||||
/* Try matching at every / boundary */
|
||||
const char *s = rel_path;
|
||||
while (*s) {
|
||||
if (glob_match(pattern, s)) {
|
||||
if (glob_match_bounded(pattern, s)) {
|
||||
return true;
|
||||
}
|
||||
const char *next = strchr(s, '/');
|
||||
@@ -359,7 +386,7 @@ int cbm_gitignore_match_result(const cbm_gitignore_t *gi, const char *rel_path,
|
||||
continue;
|
||||
}
|
||||
|
||||
bool this_match = p->rooted ? glob_match(p->pattern, rel_path)
|
||||
bool this_match = p->rooted ? glob_match_bounded(p->pattern, rel_path)
|
||||
: match_unrooted(p->pattern, rel_path, basename);
|
||||
|
||||
if (this_match) {
|
||||
|
||||
@@ -271,6 +271,23 @@ bool cbm_validate_shell_arg(const char *s) {
|
||||
return true;
|
||||
}
|
||||
|
||||
bool cbm_validate_shell_path_arg(const char *path) {
|
||||
if (!cbm_validate_shell_arg(path)) {
|
||||
return false;
|
||||
}
|
||||
#ifdef _WIN32
|
||||
/* cmd.exe expands %VAR% and, with delayed expansion, !VAR! inside the
|
||||
* double quotes these commands use; ^ is its escape character. None of the
|
||||
* three can be neutralised by quoting, so reject them outright. */
|
||||
for (const char *p = path; *p; p++) {
|
||||
if (*p == '%' || *p == '!' || *p == '^') {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
return true;
|
||||
}
|
||||
|
||||
bool cbm_validate_project_name(const char *name) {
|
||||
if (!name || !*name)
|
||||
return false;
|
||||
|
||||
@@ -57,6 +57,16 @@ char **cbm_str_split(CBMArena *a, const char *s, char delim, int *out_count);
|
||||
* Returns true if safe, false if the string contains shell metacharacters. */
|
||||
bool cbm_validate_shell_arg(const char *s);
|
||||
|
||||
/* Validate a filesystem path that will be interpolated into a shell command.
|
||||
* Everything cbm_validate_shell_arg rejects, plus the cmd.exe expansion
|
||||
* metacharacters % ! ^ on Windows: a path reaching `git -C "%s"` through
|
||||
* cmd.exe would otherwise get %VAR% / delayed-!VAR! expansion applied to it.
|
||||
*
|
||||
* Use this — not cbm_validate_shell_arg — for every path that crosses into a
|
||||
* shell command, so the three git shell-out sites cannot drift apart again.
|
||||
* Returns true if safe. */
|
||||
bool cbm_validate_shell_path_arg(const char *path);
|
||||
|
||||
/* Validate a project name is safe for file path construction.
|
||||
* Allows: alphanumeric, dash, underscore, dot (but not leading dot or dot-dot).
|
||||
* Rejects: path separators (/ \), directory traversal (..), and control chars.
|
||||
|
||||
+1
-11
@@ -40,17 +40,7 @@ static void trim_newlines(char *s) {
|
||||
}
|
||||
|
||||
static bool git_validate_repo_path(const char *repo_path) {
|
||||
if (!cbm_validate_shell_arg(repo_path)) {
|
||||
return false;
|
||||
}
|
||||
#ifdef _WIN32
|
||||
for (const char *p = repo_path; *p; p++) {
|
||||
if (*p == '%' || *p == '!' || *p == '^') {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
return true;
|
||||
return cbm_validate_shell_path_arg(repo_path);
|
||||
}
|
||||
|
||||
static int git_capture(const char *repo_path, const char *git_args, char **out) {
|
||||
|
||||
+1
-11
@@ -221,17 +221,7 @@ static int write_file_atomic(const char *path, const char *data, size_t len,
|
||||
* % ! ^. Callers then use DOUBLE quotes (honored by both POSIX sh and cmd.exe, unlike
|
||||
* single quotes on cmd.exe), so a repo path may legitimately contain spaces. */
|
||||
bool cbm_artifact_repo_path_is_shell_safe(const char *repo_path) {
|
||||
if (!cbm_validate_shell_arg(repo_path)) {
|
||||
return false;
|
||||
}
|
||||
#ifdef _WIN32
|
||||
for (const char *p = repo_path; *p; p++) {
|
||||
if (*p == '%' || *p == '!' || *p == '^') {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
#endif
|
||||
return true;
|
||||
return cbm_validate_shell_path_arg(repo_path);
|
||||
}
|
||||
|
||||
/* Get current git HEAD hash. buf must be >= CBM_SZ_64. Returns false on error. */
|
||||
|
||||
@@ -303,7 +303,11 @@ static void calls_append_args(char *props, size_t cap, const CBMCall *call) {
|
||||
n = snprintf(one, sizeof(one), "%s{\"i\":%d,\"e\":\"%s\"}", i > 0 ? "," : "", a->index,
|
||||
esc_e);
|
||||
}
|
||||
if (n <= 0 || (size_t)n >= cap - pos - PAIR_LEN) {
|
||||
/* Add rather than subtract: pos is unsigned, so `cap - pos - PAIR_LEN`
|
||||
* wraps once pos reaches cap - PAIR_LEN and stops bounding the memcpy
|
||||
* below. The closing write at the end of this function already guards
|
||||
* additively; match it. */
|
||||
if (n <= 0 || pos + (size_t)n + PAIR_LEN >= cap) {
|
||||
break; /* not enough room — close the array with what fits */
|
||||
}
|
||||
memcpy(props + pos, one, (size_t)n);
|
||||
|
||||
@@ -111,7 +111,7 @@ static int parse_git_log(const char *repo_path, commit_t **out, int *out_count)
|
||||
*out = NULL;
|
||||
*out_count = 0;
|
||||
|
||||
if (!cbm_validate_shell_arg(repo_path)) {
|
||||
if (!cbm_validate_shell_path_arg(repo_path)) {
|
||||
return CBM_NOT_FOUND;
|
||||
}
|
||||
|
||||
|
||||
+44
-10
@@ -482,8 +482,18 @@ static void handle_logs(cbm_http_conn_t *c, const cbm_http_req_t *req) {
|
||||
int start = (g_log_head - count + LOG_RING_SIZE) % LOG_RING_SIZE;
|
||||
int total = g_log_count;
|
||||
|
||||
/* Copy lines under lock */
|
||||
size_t buf_size = (size_t)count * (LOG_LINE_MAX + 10) + 64;
|
||||
/* Copy lines under lock.
|
||||
*
|
||||
* JSON escaping expands '"', '\\' and '\n' to two bytes each, so an
|
||||
* line made mostly of those serialises to roughly twice its stored length.
|
||||
* The previous budget of LOG_LINE_MAX + 10 per line under-counted that by
|
||||
* half. Ring contents come from indexer stderr, which is not escaped on
|
||||
* ingest and can legitimately contain both doubling characters — a POSIX
|
||||
* filename may.
|
||||
*
|
||||
* Budget the escaped worst case, and clamp the framing writes below anyway
|
||||
* so the size calculation is not the only thing keeping pos in range. */
|
||||
size_t buf_size = (size_t)count * (2 * LOG_LINE_MAX + 8) + 64;
|
||||
char *buf = malloc(buf_size);
|
||||
if (!buf) {
|
||||
cbm_mutex_unlock(&g_log_mutex);
|
||||
@@ -496,9 +506,9 @@ static void handle_logs(cbm_http_conn_t *c, const cbm_http_req_t *req) {
|
||||
for (int i = 0; i < count; i++) {
|
||||
int idx = (start + i) % LOG_RING_SIZE;
|
||||
if (i > 0)
|
||||
buf[pos++] = ',';
|
||||
http_appendf(buf, buf_size, &pos, ",");
|
||||
/* Escape quotes in log lines */
|
||||
buf[pos++] = '"';
|
||||
http_appendf(buf, buf_size, &pos, "\"");
|
||||
for (int j = 0; g_log_ring[idx][j] && (size_t)pos < buf_size - 10; j++) {
|
||||
char ch = g_log_ring[idx][j];
|
||||
if (ch == '"') {
|
||||
@@ -514,11 +524,19 @@ static void handle_logs(cbm_http_conn_t *c, const cbm_http_req_t *req) {
|
||||
buf[pos++] = ch;
|
||||
}
|
||||
}
|
||||
buf[pos++] = '"';
|
||||
http_appendf(buf, buf_size, &pos, "\"");
|
||||
}
|
||||
cbm_mutex_unlock(&g_log_mutex);
|
||||
http_appendf(buf, buf_size, &pos, "],\"total\":%d}", total);
|
||||
|
||||
/* http_appendf pins pos to buf_size on truncation and then writes nothing,
|
||||
* so a saturated buffer would reach the "%s" reply with no terminator in
|
||||
* range. Terminate explicitly. */
|
||||
if ((size_t)pos >= buf_size) {
|
||||
pos = (int)buf_size - 1;
|
||||
}
|
||||
buf[pos] = '\0';
|
||||
|
||||
cbm_http_replyf(c, 200, g_cors_json, "%s", buf);
|
||||
free(buf);
|
||||
}
|
||||
@@ -627,7 +645,9 @@ static void append_roots_json(char *buf, size_t bufsz, int *pos) {
|
||||
continue;
|
||||
}
|
||||
if (count++ > 0) {
|
||||
buf[(*pos)++] = ',';
|
||||
/* Once a wide listing saturates buf, http_appendf has already
|
||||
* pinned *pos to bufsz, so this separator goes through it too. */
|
||||
http_appendf(buf, bufsz, pos, ",");
|
||||
}
|
||||
http_appendf(buf, bufsz, pos, "\"%c:/\"", 'A' + i);
|
||||
}
|
||||
@@ -1089,14 +1109,28 @@ static void handle_index_status(cbm_http_server_t *server, cbm_http_conn_t *c) {
|
||||
if (st == 0)
|
||||
continue;
|
||||
if (pos > 1)
|
||||
buf[pos++] = ',';
|
||||
http_appendf(buf, sizeof(buf), &pos, ",");
|
||||
const char *ss = st == 1 ? "indexing" : st == 2 ? "done" : "error";
|
||||
/* root_path comes from POST /api/index and is up to 1023 bytes, so four
|
||||
* occupied slots exceed this buffer. http_appendf pins pos to
|
||||
* sizeof(buf) on truncation, so the separator and the close have to go
|
||||
* through it as well rather than indexing raw. Both fields are free-form,
|
||||
* so escape them — a quote in a path would otherwise end its JSON string
|
||||
* early. */
|
||||
/* Escaping can double each byte: root_path is 1024, error_msg 256. */
|
||||
char esc_path[2048];
|
||||
char esc_error[512];
|
||||
cbm_json_escape(esc_path, (int)sizeof(esc_path), server->index_jobs[i].root_path);
|
||||
cbm_json_escape(esc_error, (int)sizeof(esc_error),
|
||||
st == 3 ? server->index_jobs[i].error_msg : "");
|
||||
http_appendf(buf, sizeof(buf), &pos,
|
||||
"{\"slot\":%d,\"status\":\"%s\",\"path\":\"%s\",\"error\":\"%s\"}", i, ss,
|
||||
server->index_jobs[i].root_path,
|
||||
st == 3 ? server->index_jobs[i].error_msg : "");
|
||||
esc_path, esc_error);
|
||||
}
|
||||
http_appendf(buf, sizeof(buf), &pos, "]");
|
||||
if ((size_t)pos >= sizeof(buf)) {
|
||||
pos = (int)sizeof(buf) - 1;
|
||||
}
|
||||
buf[pos++] = ']';
|
||||
buf[pos] = '\0';
|
||||
cbm_http_replyf(c, 200, g_cors_json, "%s", buf);
|
||||
}
|
||||
|
||||
@@ -512,6 +512,58 @@ static cbm_store_t *setup_cypher_store(void) {
|
||||
return s;
|
||||
}
|
||||
|
||||
/* The query string is caller-supplied and the WHERE grammar recurses once per
|
||||
* nested '(' and once per NOT, with no depth counter between the MCP entry point
|
||||
* and the recursive descent. A few tens of KB of '(' therefore exhausted the
|
||||
* stack at parse time. Parse in a forked child so the crash surfaces as a
|
||||
* killing signal rather than taking the test runner with it; a bounded parser
|
||||
* must reject the query cleanly instead. */
|
||||
TEST(cypher_deep_nesting_rejected_not_crash) {
|
||||
#ifdef _WIN32
|
||||
SKIP_PLATFORM("fork crash-isolation is POSIX-only; the depth cap is platform-agnostic");
|
||||
#else
|
||||
enum { NEST = 30000 };
|
||||
fflush(NULL);
|
||||
pid_t pid = fork();
|
||||
if (pid == 0) {
|
||||
char *q = malloc(NEST * 2 + 64);
|
||||
if (!q) {
|
||||
_exit(2);
|
||||
}
|
||||
int n = snprintf(q, 64, "MATCH (f:Function) WHERE ");
|
||||
for (int i = 0; i < NEST; i++) {
|
||||
q[n++] = '(';
|
||||
}
|
||||
n += snprintf(q + n, 32, "f.name = \"x\"");
|
||||
for (int i = 0; i < NEST; i++) {
|
||||
q[n++] = ')';
|
||||
}
|
||||
q[n] = '\0';
|
||||
cbm_store_t *s = setup_cypher_store();
|
||||
cbm_cypher_result_t r = {0};
|
||||
/* Any clean outcome is acceptable — success or a parse error. Only a
|
||||
* crash is a failure, and that is what the signal check below catches. */
|
||||
(void)cbm_cypher_execute(s, q, "test", 0, &r);
|
||||
cbm_cypher_result_free(&r);
|
||||
cbm_store_close(s);
|
||||
free(q);
|
||||
_exit(0);
|
||||
}
|
||||
ASSERT_TRUE(pid > 0);
|
||||
int status = 0;
|
||||
(void)waitpid(pid, &status, 0);
|
||||
if (WIFSIGNALED(status)) {
|
||||
char m[96];
|
||||
snprintf(m, sizeof(m), "parser killed by signal %d — unbounded recursion depth",
|
||||
WTERMSIG(status));
|
||||
FAIL(m);
|
||||
}
|
||||
ASSERT_TRUE(WIFEXITED(status));
|
||||
ASSERT_EQ(WEXITSTATUS(status), 0);
|
||||
PASS();
|
||||
#endif
|
||||
}
|
||||
|
||||
TEST(cypher_exec_match_all_functions) {
|
||||
cbm_store_t *s = setup_cypher_store();
|
||||
cbm_cypher_result_t r = {0};
|
||||
@@ -3664,6 +3716,7 @@ SUITE(cypher) {
|
||||
/* Execution */
|
||||
RUN_TEST(cypher_exec_deadline_aborts_runaway_query_issue601);
|
||||
RUN_TEST(cypher_exec_deadline_allows_normal_query_issue601);
|
||||
RUN_TEST(cypher_deep_nesting_rejected_not_crash);
|
||||
RUN_TEST(cypher_exec_match_all_functions);
|
||||
RUN_TEST(cypher_exec_optional_empty_label_no_overflow);
|
||||
RUN_TEST(cypher_cross_join_alloc_rejects_overflow);
|
||||
|
||||
@@ -7,6 +7,8 @@
|
||||
#include "test_framework.h"
|
||||
#include "discover/discover.h"
|
||||
#include <string.h> /* strdup (test seam) */
|
||||
#include <sys/wait.h> /* fork/waitpid crash-isolation for the backtracking budget */
|
||||
#include <unistd.h>
|
||||
|
||||
/* ── Basic pattern matching ────────────────────────────────────── */
|
||||
|
||||
@@ -274,9 +276,64 @@ TEST(gi_merge_atomic_on_alloc_failure) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* `**` retries the remainder at every position, so consecutive literal + `**`
|
||||
* groups multiply and cost is exponential in the number of groups. Patterns come
|
||||
* from a committed .gitignore and every discovered path is tested against every
|
||||
* pattern, so a small ignore file can make discovery take unbounded time. The
|
||||
* matcher must give up on its step budget.
|
||||
*
|
||||
* Run in a forked child under an alarm: the alarm is only a liveness backstop —
|
||||
* the assertion is that matching TERMINATES, and the unbounded matcher never
|
||||
* reaches the exit. */
|
||||
TEST(gi_doublestar_backtracking_terminates) {
|
||||
#ifdef _WIN32
|
||||
SKIP_PLATFORM("fork/alarm crash-isolation is POSIX-only; the budget is platform-agnostic");
|
||||
#else
|
||||
fflush(NULL);
|
||||
pid_t pid = fork();
|
||||
if (pid == 0) {
|
||||
/* 20 "a**" groups then a literal that cannot match, against a run of
|
||||
* 'a' — the classic catastrophic-backtracking shape. */
|
||||
char pattern[256];
|
||||
int n = 0;
|
||||
for (int i = 0; i < 20; i++) {
|
||||
n += snprintf(pattern + n, sizeof(pattern) - (size_t)n, "a**");
|
||||
}
|
||||
(void)snprintf(pattern + n, sizeof(pattern) - (size_t)n, "X\n");
|
||||
char path[64];
|
||||
memset(path, 'a', sizeof(path) - 1);
|
||||
path[sizeof(path) - 1] = '\0';
|
||||
|
||||
alarm(20);
|
||||
cbm_gitignore_t *gi = cbm_gitignore_parse(pattern);
|
||||
if (!gi) {
|
||||
_exit(2);
|
||||
}
|
||||
/* The pattern cannot match; the only question is whether we come back. */
|
||||
(void)cbm_gitignore_matches(gi, path, false);
|
||||
cbm_gitignore_free(gi);
|
||||
_exit(0);
|
||||
}
|
||||
ASSERT_TRUE(pid > 0);
|
||||
int status = 0;
|
||||
(void)waitpid(pid, &status, 0);
|
||||
if (WIFSIGNALED(status)) {
|
||||
char m[112];
|
||||
snprintf(m, sizeof(m),
|
||||
"glob matching did not terminate (signal %d) — no backtracking budget",
|
||||
WTERMSIG(status));
|
||||
FAIL(m);
|
||||
}
|
||||
ASSERT_TRUE(WIFEXITED(status));
|
||||
ASSERT_EQ(WEXITSTATUS(status), 0);
|
||||
PASS();
|
||||
#endif
|
||||
}
|
||||
|
||||
/* ── Suite ─────────────────────────────────────────────────────── */
|
||||
|
||||
SUITE(gitignore) {
|
||||
RUN_TEST(gi_doublestar_backtracking_terminates);
|
||||
RUN_TEST(gi_empty_pattern);
|
||||
RUN_TEST(gi_exact_file);
|
||||
RUN_TEST(gi_wildcard_star);
|
||||
|
||||
@@ -2030,10 +2030,168 @@ TEST(ui_server_browse_wide_dir_no_overflow) {
|
||||
#endif
|
||||
}
|
||||
|
||||
/* The log endpoint serialises the ring into one heap buffer budgeted at
|
||||
* LOG_LINE_MAX + 10 bytes per line. JSON escaping doubles every '"' and '\\',
|
||||
* so an escape-dense line needs ~2x LOG_LINE_MAX. The inner escape loop stops
|
||||
* at buf_size - 10, but the per-line framing writes (separator comma, opening
|
||||
* quote, closing quote) were raw indexes, so every line past saturation wrote
|
||||
* three bytes beyond the allocation. Fill the ring with escape-dense lines and
|
||||
* read it back in a forked child, so the overflow surfaces as a killing signal
|
||||
* (ASan abort) instead of silent heap corruption. */
|
||||
TEST(ui_server_logs_escape_dense_no_overflow) {
|
||||
#ifdef _WIN32
|
||||
SKIP_PLATFORM("fork crash-isolation is POSIX-only; the clamp is platform-agnostic");
|
||||
#else
|
||||
fflush(NULL);
|
||||
pid_t pid = fork();
|
||||
if (pid == 0) {
|
||||
/* Worst case the ring can hold: every byte escapes to two bytes. */
|
||||
char dense[512];
|
||||
for (size_t i = 0; i < sizeof(dense) - 1; i++) {
|
||||
dense[i] = (i % 2 == 0) ? '"' : '\\';
|
||||
}
|
||||
dense[sizeof(dense) - 1] = '\0';
|
||||
for (int i = 0; i < 500; i++) { /* fills the whole 500-entry ring */
|
||||
cbm_ui_log_append(dense);
|
||||
}
|
||||
th_server_t ts;
|
||||
if (th_server_start(&ts) != 0) {
|
||||
_exit(2);
|
||||
}
|
||||
char req[256];
|
||||
int port = cbm_http_server_port(ts.srv);
|
||||
snprintf(req, sizeof(req),
|
||||
"GET /api/logs?lines=500 HTTP/1.1\r\nHost: 127.0.0.1:%d\r\n\r\n", port);
|
||||
size_t cap = 4u * 1024u * 1024u;
|
||||
char *resp = malloc(cap);
|
||||
int n = resp ? th_http(port, req, resp, (int)cap) : 0;
|
||||
int ok = (n > 0 && strstr(resp, "HTTP/1.1 200") != NULL);
|
||||
free(resp);
|
||||
th_server_stop(&ts);
|
||||
_exit(ok ? 0 : 3);
|
||||
}
|
||||
ASSERT_TRUE(pid > 0);
|
||||
int status = 0;
|
||||
(void)waitpid(pid, &status, 0);
|
||||
if (WIFSIGNALED(status)) {
|
||||
char m[96];
|
||||
snprintf(m, sizeof(m), "logs killed by signal %d — response buffer overflow",
|
||||
WTERMSIG(status));
|
||||
FAIL(m);
|
||||
}
|
||||
ASSERT_TRUE(WIFEXITED(status));
|
||||
ASSERT_EQ(WEXITSTATUS(status), 0);
|
||||
PASS();
|
||||
#endif
|
||||
}
|
||||
|
||||
/* The index-status endpoint renders every active job into a fixed 2 KB stack
|
||||
* buffer. http_appendf clamps its own writes, but the separator and the closing
|
||||
* bracket were raw indexes, so two jobs holding ~1 KB root paths (the field is
|
||||
* 1024 bytes and the value comes straight from POST /api/index) pushed pos to
|
||||
* the clamp and the close then wrote past the buffer. Drive it through the real
|
||||
* endpoint with the index executor stubbed out, in a forked child so the
|
||||
* overflow surfaces as a killing signal. */
|
||||
#define MAX_TEST_INDEX_JOBS 4
|
||||
TEST(ui_server_index_status_long_paths_no_overflow) {
|
||||
#ifdef _WIN32
|
||||
SKIP_PLATFORM("fork crash-isolation is POSIX-only; the clamp is platform-agnostic");
|
||||
#else
|
||||
char *base = th_mktempdir("cbm_status");
|
||||
if (!base) {
|
||||
FAIL("mktempdir");
|
||||
}
|
||||
/* Two real directories whose paths are long enough that two rendered job
|
||||
* entries exceed the 2 KB response buffer. */
|
||||
/* Four slots x ~520 chars overflows the 2 KB buffer while every path stays
|
||||
* well inside PATH_MAX — a single pair of ~1 KB paths would reach the
|
||||
* buffer too, but mkdir refuses them once the temp-dir prefix is added. */
|
||||
char comp[200];
|
||||
memset(comp, 'd', sizeof(comp) - 1);
|
||||
comp[sizeof(comp) - 1] = '\0';
|
||||
char deep[MAX_TEST_INDEX_JOBS][800];
|
||||
for (int j = 0; j < MAX_TEST_INDEX_JOBS; j++) {
|
||||
int n = snprintf(deep[j], sizeof(deep[j]), "%s/%d", base, j);
|
||||
while (n < 520) {
|
||||
n += snprintf(deep[j] + n, sizeof(deep[j]) - (size_t)n, "/%s", comp);
|
||||
}
|
||||
th_mkdir_p(deep[j]);
|
||||
}
|
||||
fflush(NULL);
|
||||
pid_t pid = fork();
|
||||
if (pid == 0) {
|
||||
/* The blocking executor holds every job open so all four slots are
|
||||
* occupied at once. With the non-blocking stub each job finishes
|
||||
* immediately and handle_index_start recycles the same slot, leaving a
|
||||
* single entry to render — far short of the buffer. */
|
||||
th_ui_blocking_index_executor_t executor = {0};
|
||||
atomic_init(&executor.calls, 0);
|
||||
atomic_init(&executor.release, 0);
|
||||
th_server_t ts;
|
||||
ts.srv = cbm_http_server_new(0);
|
||||
if (!ts.srv) {
|
||||
_exit(2);
|
||||
}
|
||||
cbm_http_server_set_index_executor(ts.srv, th_ui_blocking_index_executor, &executor);
|
||||
if (th_server_thread_start(&ts.tid, ts.srv) != 0) {
|
||||
_exit(2);
|
||||
}
|
||||
int port = cbm_http_server_port(ts.srv);
|
||||
for (int j = 0; j < MAX_TEST_INDEX_JOBS; j++) {
|
||||
char body[1200];
|
||||
snprintf(body, sizeof(body), "{\"root_path\":\"%s\",\"project_name\":\"p%d\"}", deep[j],
|
||||
j);
|
||||
char request[1500];
|
||||
snprintf(request, sizeof(request),
|
||||
"POST /api/index HTTP/1.1\r\nContent-Type: application/json\r\n"
|
||||
"Content-Length: %zu\r\n\r\n%s",
|
||||
strlen(body), body);
|
||||
char response[4096];
|
||||
int rn = th_http(port, request, response, sizeof(response));
|
||||
/* A rejected POST would leave the slot empty and the endpoint would
|
||||
* render nothing — a vacuous pass. Fail loudly instead. */
|
||||
if (rn <= 0 || th_status(response) != 202) {
|
||||
_exit(4);
|
||||
}
|
||||
}
|
||||
/* Wait for the state the assertion depends on — all four jobs actually
|
||||
* running — rather than for a duration. */
|
||||
if (!th_wait_atomic_int(&executor.calls, MAX_TEST_INDEX_JOBS, 5000)) {
|
||||
atomic_store(&executor.release, 1);
|
||||
_exit(5);
|
||||
}
|
||||
char req[256];
|
||||
snprintf(req, sizeof(req), "GET /api/index-status HTTP/1.1\r\nHost: 127.0.0.1:%d\r\n\r\n",
|
||||
port);
|
||||
char resp[8192];
|
||||
int n = th_http(port, req, resp, sizeof(resp));
|
||||
int ok = (n > 0 && strstr(resp, "HTTP/1.1 200") != NULL);
|
||||
atomic_store(&executor.release, 1);
|
||||
th_server_stop(&ts);
|
||||
_exit(ok ? 0 : 3);
|
||||
}
|
||||
ASSERT_TRUE(pid > 0);
|
||||
int status = 0;
|
||||
(void)waitpid(pid, &status, 0);
|
||||
th_cleanup(base);
|
||||
if (WIFSIGNALED(status)) {
|
||||
char m[96];
|
||||
snprintf(m, sizeof(m), "index-status killed by signal %d — response buffer overflow",
|
||||
WTERMSIG(status));
|
||||
FAIL(m);
|
||||
}
|
||||
ASSERT_TRUE(WIFEXITED(status));
|
||||
ASSERT_EQ(WEXITSTATUS(status), 0);
|
||||
PASS();
|
||||
#endif
|
||||
}
|
||||
|
||||
/* ── Suite ────────────────────────────────────────────────────── */
|
||||
|
||||
SUITE(httpd) {
|
||||
RUN_TEST(ui_server_browse_wide_dir_no_overflow);
|
||||
RUN_TEST(ui_server_logs_escape_dense_no_overflow);
|
||||
RUN_TEST(ui_server_index_status_long_paths_no_overflow);
|
||||
/* Parser / helpers */
|
||||
RUN_TEST(httpd_parse_simple_get);
|
||||
RUN_TEST(httpd_parse_security_headers_and_rejects_duplicates);
|
||||
|
||||
@@ -220,6 +220,30 @@ TEST(shell_rejects_newline) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Every path that reaches a shell command must go through the path variant, not
|
||||
* the bare one: the three git shell-out sites each wrap the repo path in cmd.exe
|
||||
* double quotes, where %VAR%, !VAR! and ^ stay active. One site used the bare
|
||||
* validator and so accepted them. */
|
||||
TEST(shell_path_arg_rejects_cmd_expansion) {
|
||||
/* Anything the base validator rejects, the path variant rejects too. */
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("foo'bar"));
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("$(whoami)"));
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("foo;rm -rf /"));
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg(NULL));
|
||||
#ifdef _WIN32
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("C:/repo/%USERPROFILE%"));
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("C:/repo/!PATH!"));
|
||||
ASSERT_FALSE(cbm_validate_shell_path_arg("C:/repo/a^b"));
|
||||
ASSERT_TRUE(cbm_validate_shell_path_arg("C:/Users/dev/my repo"));
|
||||
#else
|
||||
/* The cmd.exe metacharacters are inert on POSIX, so a path containing them
|
||||
* stays valid here; only the Windows build rejects them. */
|
||||
ASSERT_TRUE(cbm_validate_shell_path_arg("/tmp/repo/100%"));
|
||||
ASSERT_TRUE(cbm_validate_shell_path_arg("/tmp/my repo"));
|
||||
#endif
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(shell_rejects_carriage_return) {
|
||||
ASSERT_FALSE(cbm_validate_shell_arg("foo\rbar"));
|
||||
PASS();
|
||||
@@ -782,6 +806,7 @@ SUITE(security) {
|
||||
RUN_TEST(shell_rejects_ampersand);
|
||||
RUN_TEST(shell_rejects_backslash);
|
||||
RUN_TEST(shell_rejects_newline);
|
||||
RUN_TEST(shell_path_arg_rejects_cmd_expansion);
|
||||
RUN_TEST(shell_rejects_carriage_return);
|
||||
RUN_TEST(shell_rejects_null);
|
||||
RUN_TEST(shell_rejects_double_quote);
|
||||
|
||||
Reference in new issue
Block a user