diff --git a/internal/cbm/extract_calls.c b/internal/cbm/extract_calls.c index be739d717..25877d3d1 100644 --- a/internal/cbm/extract_calls.c +++ b/internal/cbm/extract_calls.c @@ -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")); diff --git a/src/cypher/cypher.c b/src/cypher/cypher.c index 7f1d7854e..571ae2555 100644 --- a/src/cypher/cypher.c +++ b/src/cypher/cypher.c @@ -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); diff --git a/src/discover/gitignore.c b/src/discover/gitignore.c index d390c637f..1da8814c6 100644 --- a/src/discover/gitignore.c +++ b/src/discover/gitignore.c @@ -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) { diff --git a/src/foundation/str_util.c b/src/foundation/str_util.c index 26542e927..c8aec01af 100644 --- a/src/foundation/str_util.c +++ b/src/foundation/str_util.c @@ -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; diff --git a/src/foundation/str_util.h b/src/foundation/str_util.h index da02c760f..e0ff46057 100644 --- a/src/foundation/str_util.h +++ b/src/foundation/str_util.h @@ -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. diff --git a/src/git/git_context.c b/src/git/git_context.c index 14f48714d..f739c46e6 100644 --- a/src/git/git_context.c +++ b/src/git/git_context.c @@ -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) { diff --git a/src/pipeline/artifact.c b/src/pipeline/artifact.c index 7d57db895..66094b4f7 100644 --- a/src/pipeline/artifact.c +++ b/src/pipeline/artifact.c @@ -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. */ diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index 591ed5594..7eafdda1c 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -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); diff --git a/src/pipeline/pass_githistory.c b/src/pipeline/pass_githistory.c index 80ad83d48..554acea2f 100644 --- a/src/pipeline/pass_githistory.c +++ b/src/pipeline/pass_githistory.c @@ -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; } diff --git a/src/ui/http_server.c b/src/ui/http_server.c index 6eea91887..5e734d0a4 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -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); } diff --git a/tests/test_cypher.c b/tests/test_cypher.c index f103d2a50..d8f490f83 100644 --- a/tests/test_cypher.c +++ b/tests/test_cypher.c @@ -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); diff --git a/tests/test_gitignore.c b/tests/test_gitignore.c index 8cffc0aba..0541e7fcc 100644 --- a/tests/test_gitignore.c +++ b/tests/test_gitignore.c @@ -7,6 +7,8 @@ #include "test_framework.h" #include "discover/discover.h" #include /* strdup (test seam) */ +#include /* fork/waitpid crash-isolation for the backtracking budget */ +#include /* ── 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); diff --git a/tests/test_httpd.c b/tests/test_httpd.c index 5f18696e3..e3a3dc080 100644 --- a/tests/test_httpd.c +++ b/tests/test_httpd.c @@ -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); diff --git a/tests/test_security.c b/tests/test_security.c index 34676620e..8f2731984 100644 --- a/tests/test_security.c +++ b/tests/test_security.c @@ -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);