Skip to content

Commit 822a56f

Browse files
fix(mcp): fix use-after-free in handle_manage_adr get path
yyjson_mut_obj_add_str stores the raw pointer without copying. Freeing buf before yy_doc_to_str serializes the document causes yyjson to read freed heap memory, producing garbage JSON. cbm_jsonrpc_format_response then fails to parse the result field (res_doc == NULL), so "result" is omitted from the JSON-RPC response and the MCP client hangs indefinitely. Fix: hoist adr_buf to function scope (initialized NULL), remove the premature free, and free it after yy_doc_to_str has serialized the doc. Adds regression test tool_manage_adr_get_with_existing_adr that FAILS before this fix and PASSES after it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 75b5dcb commit 822a56f

File tree

2 files changed

+65
-5
lines changed

2 files changed

+65
-5
lines changed

src/mcp/mcp.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1897,6 +1897,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
18971897
char adr_path[4096];
18981898
snprintf(adr_path, sizeof(adr_path), "%s/adr.md", adr_dir);
18991899

1900+
char *adr_buf = NULL; /* freed after yy_doc_to_str — yyjson holds pointer, not copy */
19001901
yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
19011902
yyjson_mut_val *root_obj = yyjson_mut_obj(doc);
19021903
yyjson_mut_doc_set_root(doc, root_obj);
@@ -1937,12 +1938,12 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
19371938
(void)fseek(fp, 0, SEEK_END);
19381939
long sz = ftell(fp);
19391940
(void)fseek(fp, 0, SEEK_SET);
1940-
char *buf = malloc(sz + 1);
1941-
size_t n = fread(buf, 1, sz, fp);
1942-
buf[n] = '\0';
1941+
adr_buf = malloc(sz + 1);
1942+
size_t n = fread(adr_buf, 1, sz, fp);
1943+
adr_buf[n] = '\0';
19431944
(void)fclose(fp);
1944-
yyjson_mut_obj_add_str(doc, root_obj, "content", buf);
1945-
free(buf);
1945+
yyjson_mut_obj_add_str(doc, root_obj, "content", adr_buf);
1946+
/* do NOT free adr_buf here: yyjson stores the pointer, not a copy */
19461947
} else {
19471948
yyjson_mut_obj_add_str(doc, root_obj, "content", "");
19481949
yyjson_mut_obj_add_str(doc, root_obj, "status", "no_adr");
@@ -1959,6 +1960,7 @@ static char *handle_manage_adr(cbm_mcp_server_t *srv, const char *args) {
19591960

19601961
char *json = yy_doc_to_str(doc);
19611962
yyjson_mut_doc_free(doc);
1963+
free(adr_buf); /* safe to free now — doc has been serialized */
19621964
free(root_path);
19631965
free(project);
19641966
free(mode_str);

tests/test_mcp.c

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,63 @@ TEST(tool_manage_adr_no_project) {
614614
PASS();
615615
}
616616

617+
/* Regression test for use-after-free in handle_manage_adr (get path).
618+
* MUST FAIL before fix: free(buf) is called before yy_doc_to_str serializes doc,
619+
* so result field is missing or contains garbage. MUST PASS after fix. */
620+
TEST(tool_manage_adr_get_with_existing_adr) {
621+
/* Create a temp directory with .codebase-memory/adr.md */
622+
char tmp_dir[256];
623+
snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-adr-test-XXXXXX");
624+
if (!cbm_mkdtemp(tmp_dir)) {
625+
PASS(); /* skip if mkdtemp fails */
626+
}
627+
628+
char adr_dir[512];
629+
snprintf(adr_dir, sizeof(adr_dir), "%s/.codebase-memory", tmp_dir);
630+
cbm_mkdir(adr_dir);
631+
632+
char adr_path[512];
633+
snprintf(adr_path, sizeof(adr_path), "%s/adr.md", adr_dir);
634+
FILE *fp = fopen(adr_path, "w");
635+
ASSERT_NOT_NULL(fp);
636+
fputs("## PURPOSE\nTest ADR content for regression test.\n\n"
637+
"## STACK\nC, SQLite.\n\n"
638+
"## ARCHITECTURE\nMCP server.\n",
639+
fp);
640+
fclose(fp);
641+
642+
/* Create server and register the project */
643+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
644+
ASSERT_NOT_NULL(srv);
645+
cbm_store_t *st = cbm_mcp_server_store(srv);
646+
ASSERT_NOT_NULL(st);
647+
cbm_store_upsert_project(st, "test-adr-uaf", tmp_dir);
648+
cbm_mcp_server_set_project(srv, "test-adr-uaf");
649+
650+
/* Call manage_adr via full JSON-RPC path to exercise cbm_jsonrpc_format_response.
651+
* The bug: free(buf) before yy_doc_to_str causes garbage JSON; format_response
652+
* then fails to parse the result and omits the "result" field entirely. */
653+
char *resp = cbm_mcp_server_handle(
654+
srv, "{\"jsonrpc\":\"2.0\",\"id\":99,\"method\":\"tools/call\","
655+
"\"params\":{\"name\":\"manage_adr\","
656+
"\"arguments\":{\"project\":\"test-adr-uaf\",\"mode\":\"get\"}}}");
657+
ASSERT_NOT_NULL(resp);
658+
/* JSON-RPC response must include a "result" field (absent when use-after-free) */
659+
ASSERT_NOT_NULL(strstr(resp, "\"result\""));
660+
/* ADR content must appear in response */
661+
ASSERT_NOT_NULL(strstr(resp, "PURPOSE"));
662+
/* Must not be an error */
663+
ASSERT_NULL(strstr(resp, "isError"));
664+
free(resp);
665+
666+
/* Clean up */
667+
cbm_mcp_server_free(srv);
668+
remove(adr_path);
669+
rmdir(adr_dir);
670+
rmdir(tmp_dir);
671+
PASS();
672+
}
673+
617674
TEST(tool_ingest_traces_basic) {
618675
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
619676

@@ -1273,6 +1330,7 @@ SUITE(mcp) {
12731330
RUN_TEST(tool_search_code_no_project);
12741331
RUN_TEST(tool_detect_changes_no_project);
12751332
RUN_TEST(tool_manage_adr_no_project);
1333+
RUN_TEST(tool_manage_adr_get_with_existing_adr);
12761334
RUN_TEST(tool_ingest_traces_basic);
12771335
RUN_TEST(tool_ingest_traces_empty);
12781336

0 commit comments

Comments
 (0)