mcp: reject an option-like base_branch in detect_changes

base_branch is spliced into `git diff --name-only "<base>"...HEAD`. The
existing check rejected shell metacharacters but not a leading '-', so a
value like --output=<path> was passed to git as an option and wrote the diff
to an arbitrary file. A real git ref never starts with '-'; reject it
alongside the metacharacter check.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-07-07 21:18:55 +02:00
1 parent 7071310825
commit 72e6a14adb
2 files changed
+24 -2

No files matched your search

+6 -2
View File
@@ -5381,8 +5381,12 @@ static char *handle_detect_changes(cbm_mcp_server_t *srv, const char *args) {
base_branch = heap_strdup("main");
}
/* Reject shell metacharacters in user-supplied branch name */
if (!cbm_validate_shell_arg(base_branch)) {
/* Reject shell metacharacters, and a leading '-', in the user-supplied
* branch name. base_branch is spliced into `git diff --name-only
* "<base>"...HEAD`; a value starting with '-' would be read by git as an
* option rather than a ref (e.g. `--output=<path>` writes the diff to an
* arbitrary file). A real git ref never begins with '-'. */
if (!cbm_validate_shell_arg(base_branch) || base_branch[0] == '-') {
free(project);
free(base_branch);
free(scope);
+18
View File
@@ -4912,12 +4912,30 @@ TEST(mcp_path_within_root_rejects_escape) {
#endif
}
/* base_branch is spliced into a `git diff --name-only "<base>"...HEAD` command;
* a value starting with '-' would be taken by git as an option (e.g.
* --output=<path> writes the diff to an arbitrary file) rather than a ref. It
* must be rejected up front, alongside the shell-metacharacter check. */
TEST(detect_changes_rejects_option_like_base_branch) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
char *resp = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":77,\"method\":\"tools/call\","
"\"params\":{\"name\":\"detect_changes\","
"\"arguments\":{\"project\":\"p\",\"base_branch\":\"--output=/tmp/cbm_pwn\"}}}");
ASSERT_NOT_NULL(resp);
ASSERT_NOT_NULL(strstr(resp, "invalid characters"));
free(resp);
cbm_mcp_server_free(srv);
PASS();
}
/* ══════════════════════════════════════════════════════════════════
* SUITE
* ══════════════════════════════════════════════════════════════════ */
SUITE(mcp) {
RUN_TEST(mcp_path_within_root_rejects_escape);
RUN_TEST(detect_changes_rejects_option_like_base_branch);
/* JSON-RPC parsing */
RUN_TEST(jsonrpc_parse_request);
RUN_TEST(jsonrpc_parse_notification);