fix: security and correctness hardening from codebase audit

- str_util: add cbm_validate_project_name() — strict allowlist
  ([A-Za-z0-9._-], no leading dot, no '..', no path separators) so a
  project name can never escape the cache directory.
- store/mcp/http_server: validate project names (fail-closed) before
  interpolating them into .db file paths (path-traversal hardening;
  defense-in-depth for the wrong-path class in #331).
- store: fix use-after-free in the LIKE pool — like_pool_add() frees its
  argument when the pool is full, so capture pool_was_full before the add
  and skip the subsequent bind that would touch the freed pointer (two
  call sites). Also fix the find_edges_by_url_path LIKE pattern (stray
  double-percent) and add NULL guards to cbm_store_checkpoint /
  cbm_store_node_degree / cbm_store_restore_from.
- mcp: emit "result": null when a JSON-RPC response would otherwise
  carry neither result nor error (2.0 compliance); bounds-check the
  project-name accumulator against truncation.
- http_server: JSON-escape directory and parent names in the browse
  endpoint to prevent stored-XSS/JSON injection via crafted names.
- cypher: expr_free now recurses on EXPR_FREE_STACK overflow instead of
  silently dropping subtrees (memory leak).

Distilled from #352 onto current main (store.c like-pool hunk re-resolved
against the v0.7.0 search refactor; restored cypher.c EOF newline).
This commit is contained in:
Code Analyzer
2026-05-30 17:19:56 +02:00
committed by Martin Vogel
parent 9d6e28bccf
commit 21c73b57f1
6 changed files with 88 additions and 11 deletions
+12 -4
View File
@@ -742,11 +742,19 @@ static void expr_free(cbm_expr_t *e) {
}
free(cur->cond.in_values);
}
if (cur->right && top < EXPR_FREE_STACK) {
stack[top++] = cur->right;
if (cur->right) {
if (top < EXPR_FREE_STACK) {
stack[top++] = cur->right;
} else {
expr_free(cur->right); /* recurse when stack overflows */
}
}
if (cur->left && top < EXPR_FREE_STACK) {
stack[top++] = cur->left;
if (cur->left) {
if (top < EXPR_FREE_STACK) {
stack[top++] = cur->left;
} else {
expr_free(cur->left); /* recurse when stack overflows */
}
}
free(cur);
}
+18
View File
@@ -270,6 +270,24 @@ bool cbm_validate_shell_arg(const char *s) {
return true;
}
bool cbm_validate_project_name(const char *name) {
if (!name || !*name) return false;
/* Reject directory traversal */
if (strcmp(name, "..") == 0 || strstr(name, "..") != NULL) return false;
/* Reject path separators */
if (strchr(name, '/') || strchr(name, '\\')) return false;
/* Reject leading dot (hidden files / relative refs) */
if (name[0] == '.') return false;
/* Allow only alphanumeric, dash, underscore, dot */
for (const char *p = name; *p; p++) {
if (!(((*p >= 'a') && (*p <= 'z')) || ((*p >= 'A') && (*p <= 'Z')) ||
((*p >= '0') && (*p <= '9')) || *p == '-' || *p == '_' || *p == '.')) {
return false;
}
}
return true;
}
int cbm_json_escape(char *buf, int bufsize, const char *src) {
if (!buf || bufsize <= 0) {
return 0;
+6
View File
@@ -57,6 +57,12 @@ 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 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.
* Returns true if safe, false if the name could escape the cache directory. */
bool cbm_validate_project_name(const char *name);
/* Safe snprintf append: clamps offset to prevent buffer overflow on truncation.
* When snprintf truncates, it returns what it WOULD have written, which can make
* offset > bufsize. Next call: bufsize - offset wraps unsigned → huge → overflow.
+11
View File
@@ -201,6 +201,9 @@ char *cbm_jsonrpc_format_response(const cbm_jsonrpc_response_t *resp) {
yyjson_mut_obj_add_val(doc, root, "result", res_val);
yyjson_doc_free(res_doc);
}
} else {
/* JSON-RPC 2.0 spec: response MUST contain "result" or "error" */
yyjson_mut_obj_add_null(doc, root, "result");
}
char *out = yy_doc_to_str(doc);
@@ -722,6 +725,10 @@ static const char *cache_dir(char *buf, size_t bufsz) {
/* Returns full .db path for a project: <cache_dir>/<project>.db */
static const char *project_db_path(const char *project, char *buf, size_t bufsz) {
if (!cbm_validate_project_name(project)) {
buf[0] = '\0';
return buf;
}
char dir[CBM_SZ_1K];
cache_dir(dir, sizeof(dir));
snprintf(buf, bufsz, "%s/%s.db", dir, project);
@@ -815,12 +822,16 @@ static int collect_db_project_names(const char *dir_path, char *out, size_t out_
if (!is_project_db_file(n, len)) {
continue;
}
if ((size_t)offset >= out_sz) break; /* bounds check before write */
if (count > 0 && offset < (int)out_sz - MCP_SEPARATOR) {
out[offset++] = ',';
}
int wrote = snprintf(out + offset, out_sz - (size_t)offset, "\"%.*s\"", (int)(len - 3), n);
if (wrote > 0) {
offset += wrote;
if ((size_t)offset >= out_sz) {
offset = (int)out_sz - 1; /* clamp on truncation */
}
}
count++;
}
+24 -4
View File
@@ -63,6 +63,7 @@ enum {
#include "foundation/compat.h"
#include "foundation/log.h"
#include "foundation/compat_regex.h"
#include "foundation/str_util.h"
#define XXH_INLINE_ALL
#include "xxhash/xxhash.h"
@@ -704,6 +705,9 @@ cbm_store_t *cbm_store_open(const char *project) {
if (!project) {
return NULL;
}
if (!cbm_validate_project_name(project)) {
return NULL;
}
const char *cdir = cbm_resolve_cache_dir();
if (!cdir) {
cdir = cbm_tmpdir();
@@ -835,6 +839,9 @@ int cbm_store_create_indexes(cbm_store_t *s) {
/* ── Checkpoint ─────────────────────────────────────────────────── */
int cbm_store_checkpoint(cbm_store_t *s) {
if (!s) {
return CBM_STORE_ERR;
}
/* PASSIVE never blocks readers and never ftruncate()s either file.
* SQLite recommends PASSIVE for shared databases — TRUNCATE shrinks
* the WAL via ftruncate(fd, 0) on success, which on macOS can raise
@@ -1754,6 +1761,11 @@ int cbm_store_find_nodes_by_qn_suffix(cbm_store_t *s, const char *project, const
/* ── NodeDegree ────────────────────────────────────────────────── */
void cbm_store_node_degree(cbm_store_t *s, int64_t node_id, int *in_deg, int *out_deg) {
if (!s) {
if (in_deg) *in_deg = 0;
if (out_deg) *out_deg = 0;
return;
}
*in_deg = 0;
*out_deg = 0;
@@ -1982,7 +1994,7 @@ int cbm_store_find_edges_by_url_path(cbm_store_t *s, const char *project, const
/* Search properties JSON for url_path containing keyword */
char like_pattern[CBM_SZ_512];
snprintf(like_pattern, sizeof(like_pattern), "%%\"url_path\":\"%%%%%s%%%%\"%%", keyword);
snprintf(like_pattern, sizeof(like_pattern), "%%\"url_path\":\"%%%s%%\"%%", keyword);
const char *sql = "SELECT id, project, source_id, target_id, type, properties FROM edges "
"WHERE project = ?1 AND properties LIKE ?2";
@@ -2018,6 +2030,9 @@ int cbm_store_find_edges_by_url_path(cbm_store_t *s, const char *project, const
/* ── RestoreFrom ───────────────────────────────────────────────── */
int cbm_store_restore_from(cbm_store_t *dst, cbm_store_t *src) {
if (!dst || !src) {
return CBM_STORE_ERR;
}
sqlite3_backup *bk = sqlite3_backup_init(dst->db, "main", src->db, "main");
if (!bk) {
store_set_error_sqlite(dst, "backup init");
@@ -2292,7 +2307,9 @@ static void where_add_like_hints(const char *column, const char *pattern, char *
free(hints[i]);
if (!lp)
continue;
int pool_was_full = (pool->count >= ST_LIKE_POOL_MAX);
like_pool_add(pool, lp);
if (pool_was_full) continue; /* lp was freed — skip bind */
snprintf(bind_buf, sizeof(bind_buf), "%s LIKE ?%d", column, *bind_idx + SKIP_ONE);
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
where_bind_text(binds, bind_idx, lp);
@@ -2329,10 +2346,13 @@ static int search_where_basic(const cbm_search_params_t *params, char *where, in
}
if (params->file_pattern) {
char *lp = cbm_glob_to_like(params->file_pattern);
int pool_was_full = (pool->count >= ST_LIKE_POOL_MAX);
like_pool_add(pool, lp);
snprintf(bind_buf, sizeof(bind_buf), "n.file_path LIKE ?%d", *bind_idx + SKIP_ONE);
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
where_bind_text(binds, bind_idx, lp);
if (!pool_was_full && lp) {
snprintf(bind_buf, sizeof(bind_buf), "n.file_path LIKE ?%d", *bind_idx + SKIP_ONE);
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
where_bind_text(binds, bind_idx, lp);
}
}
return *nparams;
}
+17 -3
View File
@@ -20,6 +20,7 @@
#include "foundation/log.h"
#include "foundation/platform.h"
#include "foundation/compat.h"
#include "foundation/str_util.h"
#include "foundation/compat_thread.h"
#include <mongoose/mongoose.h>
@@ -126,6 +127,10 @@ static bool get_query_param(struct mg_str query, const char *name, char *buf, in
/* Build DB path for a project: <cache_dir>/<project>.db */
static void db_path_for_project(const char *project, char *buf, size_t bufsz) {
if (!cbm_validate_project_name(project)) {
buf[0] = '\0';
return;
}
const char *dir = cbm_resolve_cache_dir();
if (!dir) {
dir = cbm_tmpdir();
@@ -444,7 +449,12 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
if (count > 0)
buf[pos++] = ',';
pos += snprintf(buf + pos, sizeof(buf) - (size_t)pos, "\"%s\"", ent->d_name);
/* Escape directory name to prevent XSS (e.g., names with quotes/angle brackets) */
{
char esc[512];
cbm_json_escape(esc, (int)sizeof(esc), ent->d_name);
pos += snprintf(buf + pos, sizeof(buf) - (size_t)pos, "\"%s\"", esc);
}
if (pos >= (int)sizeof(buf)) {
pos = (int)sizeof(buf) - 1;
}
@@ -455,7 +465,7 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
}
closedir(dir);
/* Parent path */
/* Parent path — escape to prevent injection */
char parent[1024];
snprintf(parent, sizeof(parent), "%s", path);
char *last_slash = strrchr(parent, '/');
@@ -464,7 +474,11 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
else
snprintf(parent, sizeof(parent), "/");
pos += snprintf(buf + pos, sizeof(buf) - (size_t)pos, "],\"parent\":\"%s\"}", parent);
{
char esc_parent[2048];
cbm_json_escape(esc_parent, (int)sizeof(esc_parent), parent);
pos += snprintf(buf + pos, sizeof(buf) - (size_t)pos, "],\"parent\":\"%s\"}", esc_parent);
}
mg_http_reply(c, 200, g_cors_json, "%s", buf);
}