mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-06 08:35:21 +08:00
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:
committed by
Martin Vogel
parent
9d6e28bccf
commit
21c73b57f1
+12
-4
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
@@ -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
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user