mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-09 09:11:00 +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);
|
free(cur->cond.in_values);
|
||||||
}
|
}
|
||||||
if (cur->right && top < EXPR_FREE_STACK) {
|
if (cur->right) {
|
||||||
stack[top++] = 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) {
|
if (cur->left) {
|
||||||
stack[top++] = cur->left;
|
if (top < EXPR_FREE_STACK) {
|
||||||
|
stack[top++] = cur->left;
|
||||||
|
} else {
|
||||||
|
expr_free(cur->left); /* recurse when stack overflows */
|
||||||
|
}
|
||||||
}
|
}
|
||||||
free(cur);
|
free(cur);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -270,6 +270,24 @@ bool cbm_validate_shell_arg(const char *s) {
|
|||||||
return true;
|
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) {
|
int cbm_json_escape(char *buf, int bufsize, const char *src) {
|
||||||
if (!buf || bufsize <= 0) {
|
if (!buf || bufsize <= 0) {
|
||||||
return 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. */
|
* Returns true if safe, false if the string contains shell metacharacters. */
|
||||||
bool cbm_validate_shell_arg(const char *s);
|
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.
|
/* Safe snprintf append: clamps offset to prevent buffer overflow on truncation.
|
||||||
* When snprintf truncates, it returns what it WOULD have written, which can make
|
* When snprintf truncates, it returns what it WOULD have written, which can make
|
||||||
* offset > bufsize. Next call: bufsize - offset wraps unsigned → huge → overflow.
|
* 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_mut_obj_add_val(doc, root, "result", res_val);
|
||||||
yyjson_doc_free(res_doc);
|
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);
|
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 */
|
/* 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) {
|
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];
|
char dir[CBM_SZ_1K];
|
||||||
cache_dir(dir, sizeof(dir));
|
cache_dir(dir, sizeof(dir));
|
||||||
snprintf(buf, bufsz, "%s/%s.db", dir, project);
|
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)) {
|
if (!is_project_db_file(n, len)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
if ((size_t)offset >= out_sz) break; /* bounds check before write */
|
||||||
if (count > 0 && offset < (int)out_sz - MCP_SEPARATOR) {
|
if (count > 0 && offset < (int)out_sz - MCP_SEPARATOR) {
|
||||||
out[offset++] = ',';
|
out[offset++] = ',';
|
||||||
}
|
}
|
||||||
int wrote = snprintf(out + offset, out_sz - (size_t)offset, "\"%.*s\"", (int)(len - 3), n);
|
int wrote = snprintf(out + offset, out_sz - (size_t)offset, "\"%.*s\"", (int)(len - 3), n);
|
||||||
if (wrote > 0) {
|
if (wrote > 0) {
|
||||||
offset += wrote;
|
offset += wrote;
|
||||||
|
if ((size_t)offset >= out_sz) {
|
||||||
|
offset = (int)out_sz - 1; /* clamp on truncation */
|
||||||
|
}
|
||||||
}
|
}
|
||||||
count++;
|
count++;
|
||||||
}
|
}
|
||||||
|
|||||||
+24
-4
@@ -63,6 +63,7 @@ enum {
|
|||||||
#include "foundation/compat.h"
|
#include "foundation/compat.h"
|
||||||
#include "foundation/log.h"
|
#include "foundation/log.h"
|
||||||
#include "foundation/compat_regex.h"
|
#include "foundation/compat_regex.h"
|
||||||
|
#include "foundation/str_util.h"
|
||||||
|
|
||||||
#define XXH_INLINE_ALL
|
#define XXH_INLINE_ALL
|
||||||
#include "xxhash/xxhash.h"
|
#include "xxhash/xxhash.h"
|
||||||
@@ -704,6 +705,9 @@ cbm_store_t *cbm_store_open(const char *project) {
|
|||||||
if (!project) {
|
if (!project) {
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
if (!cbm_validate_project_name(project)) {
|
||||||
|
return NULL;
|
||||||
|
}
|
||||||
const char *cdir = cbm_resolve_cache_dir();
|
const char *cdir = cbm_resolve_cache_dir();
|
||||||
if (!cdir) {
|
if (!cdir) {
|
||||||
cdir = cbm_tmpdir();
|
cdir = cbm_tmpdir();
|
||||||
@@ -835,6 +839,9 @@ int cbm_store_create_indexes(cbm_store_t *s) {
|
|||||||
/* ── Checkpoint ─────────────────────────────────────────────────── */
|
/* ── Checkpoint ─────────────────────────────────────────────────── */
|
||||||
|
|
||||||
int cbm_store_checkpoint(cbm_store_t *s) {
|
int cbm_store_checkpoint(cbm_store_t *s) {
|
||||||
|
if (!s) {
|
||||||
|
return CBM_STORE_ERR;
|
||||||
|
}
|
||||||
/* PASSIVE never blocks readers and never ftruncate()s either file.
|
/* PASSIVE never blocks readers and never ftruncate()s either file.
|
||||||
* SQLite recommends PASSIVE for shared databases — TRUNCATE shrinks
|
* SQLite recommends PASSIVE for shared databases — TRUNCATE shrinks
|
||||||
* the WAL via ftruncate(fd, 0) on success, which on macOS can raise
|
* 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 ────────────────────────────────────────────────── */
|
/* ── NodeDegree ────────────────────────────────────────────────── */
|
||||||
|
|
||||||
void cbm_store_node_degree(cbm_store_t *s, int64_t node_id, int *in_deg, int *out_deg) {
|
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;
|
*in_deg = 0;
|
||||||
*out_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 */
|
/* Search properties JSON for url_path containing keyword */
|
||||||
char like_pattern[CBM_SZ_512];
|
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 "
|
const char *sql = "SELECT id, project, source_id, target_id, type, properties FROM edges "
|
||||||
"WHERE project = ?1 AND properties LIKE ?2";
|
"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 ───────────────────────────────────────────────── */
|
/* ── RestoreFrom ───────────────────────────────────────────────── */
|
||||||
|
|
||||||
int cbm_store_restore_from(cbm_store_t *dst, cbm_store_t *src) {
|
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");
|
sqlite3_backup *bk = sqlite3_backup_init(dst->db, "main", src->db, "main");
|
||||||
if (!bk) {
|
if (!bk) {
|
||||||
store_set_error_sqlite(dst, "backup init");
|
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]);
|
free(hints[i]);
|
||||||
if (!lp)
|
if (!lp)
|
||||||
continue;
|
continue;
|
||||||
|
int pool_was_full = (pool->count >= ST_LIKE_POOL_MAX);
|
||||||
like_pool_add(pool, lp);
|
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);
|
snprintf(bind_buf, sizeof(bind_buf), "%s LIKE ?%d", column, *bind_idx + SKIP_ONE);
|
||||||
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
|
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
|
||||||
where_bind_text(binds, bind_idx, lp);
|
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) {
|
if (params->file_pattern) {
|
||||||
char *lp = cbm_glob_to_like(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);
|
like_pool_add(pool, lp);
|
||||||
snprintf(bind_buf, sizeof(bind_buf), "n.file_path LIKE ?%d", *bind_idx + SKIP_ONE);
|
if (!pool_was_full && lp) {
|
||||||
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
|
snprintf(bind_buf, sizeof(bind_buf), "n.file_path LIKE ?%d", *bind_idx + SKIP_ONE);
|
||||||
where_bind_text(binds, bind_idx, lp);
|
*wlen = where_append(where, where_sz, *wlen, nparams, bind_buf);
|
||||||
|
where_bind_text(binds, bind_idx, lp);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return *nparams;
|
return *nparams;
|
||||||
}
|
}
|
||||||
|
|||||||
+17
-3
@@ -20,6 +20,7 @@
|
|||||||
#include "foundation/log.h"
|
#include "foundation/log.h"
|
||||||
#include "foundation/platform.h"
|
#include "foundation/platform.h"
|
||||||
#include "foundation/compat.h"
|
#include "foundation/compat.h"
|
||||||
|
#include "foundation/str_util.h"
|
||||||
#include "foundation/compat_thread.h"
|
#include "foundation/compat_thread.h"
|
||||||
|
|
||||||
#include <mongoose/mongoose.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 */
|
/* Build DB path for a project: <cache_dir>/<project>.db */
|
||||||
static void db_path_for_project(const char *project, char *buf, size_t bufsz) {
|
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();
|
const char *dir = cbm_resolve_cache_dir();
|
||||||
if (!dir) {
|
if (!dir) {
|
||||||
dir = cbm_tmpdir();
|
dir = cbm_tmpdir();
|
||||||
@@ -444,7 +449,12 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
|
|||||||
|
|
||||||
if (count > 0)
|
if (count > 0)
|
||||||
buf[pos++] = ',';
|
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)) {
|
if (pos >= (int)sizeof(buf)) {
|
||||||
pos = (int)sizeof(buf) - 1;
|
pos = (int)sizeof(buf) - 1;
|
||||||
}
|
}
|
||||||
@@ -455,7 +465,7 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
|
|||||||
}
|
}
|
||||||
closedir(dir);
|
closedir(dir);
|
||||||
|
|
||||||
/* Parent path */
|
/* Parent path — escape to prevent injection */
|
||||||
char parent[1024];
|
char parent[1024];
|
||||||
snprintf(parent, sizeof(parent), "%s", path);
|
snprintf(parent, sizeof(parent), "%s", path);
|
||||||
char *last_slash = strrchr(parent, '/');
|
char *last_slash = strrchr(parent, '/');
|
||||||
@@ -464,7 +474,11 @@ static void handle_browse(struct mg_connection *c, struct mg_http_message *hm) {
|
|||||||
else
|
else
|
||||||
snprintf(parent, sizeof(parent), "/");
|
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);
|
mg_http_reply(c, 200, g_cors_json, "%s", buf);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user