Skip to content

Commit eca433b

Browse files
committed
fix(security): block " < > in cbm_validate_shell_arg
The Windows search path landed in 82a9052 wraps shell args in cmd.exe- level "powershell -Command \"...'%s'...\"". PowerShell's single quotes hold the inner '%s' interpolation, but a literal " inside the user- supplied value can close the cmd.exe outer quote. With ' ; | & $ ` already blocked, that wouldn't reach RCE on its own — but < and > were unblocked, so a quote-break followed by cmd.exe redirection (e.g. *">C:\evil.txt") would expose a file-write primitive. Block " < > unconditionally. The validator's contract is "safe inside single quotes for shell interpolation"; on POSIX these aren't strictly necessary (single quotes hold), but on Windows the cmd.exe→powershell wrapping makes them load-bearing, and unconditional blocking keeps the validator simple — no ifdef-laddered policy and no surprises if a future caller invokes it from a different shell context. Three tests added to tests/test_security.c covering each new char.
1 parent 82a9052 commit eca433b

3 files changed

Lines changed: 29 additions & 1 deletion

File tree

src/foundation/str_util.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,14 @@ bool cbm_validate_shell_arg(const char *s) {
249249
for (const char *p = s; *p; p++) {
250250
switch (*p) {
251251
case '\'':
252+
case '"':
252253
case ';':
253254
case '|':
254255
case '&':
255256
case '$':
256257
case '`':
258+
case '<':
259+
case '>':
257260
case '\n':
258261
case '\r':
259262
#ifndef _WIN32

src/foundation/str_util.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ char *cbm_str_strip_ext(CBMArena *a, const char *path);
4949
char **cbm_str_split(CBMArena *a, const char *s, char delim, int *out_count);
5050

5151
/* Validate a string is safe for shell interpolation inside single quotes.
52-
* Rejects: ' ; | & $ ` \n \r \0 (embedded NULs via len check).
52+
* Rejects: ' " ; | & $ ` < > \n \r \0 (embedded NULs via len check).
53+
* The Windows search path wraps shell args in cmd.exe-level "powershell -Command
54+
* \"...'%s'...\"", so " can close the cmd.exe outer quote even if PowerShell's
55+
* single quotes hold; < > would then become cmd.exe redirection (file-write
56+
* primitive). Blocking these unconditionally hardens both POSIX and Windows.
5357
* Returns true if safe, false if the string contains shell metacharacters. */
5458
bool cbm_validate_shell_arg(const char *s);
5559

tests/test_security.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,24 @@ TEST(shell_rejects_null) {
7575
PASS();
7676
}
7777

78+
TEST(shell_rejects_double_quote) {
79+
/* On Windows, the search code path wraps args in cmd.exe-level
80+
* "powershell -Command \"...'%s'...\"". A " in the input would close
81+
* the cmd.exe outer quote. Block unconditionally. */
82+
ASSERT_FALSE(cbm_validate_shell_arg("foo\"bar"));
83+
PASS();
84+
}
85+
86+
TEST(shell_rejects_redirect_out) {
87+
ASSERT_FALSE(cbm_validate_shell_arg("foo>out.txt"));
88+
PASS();
89+
}
90+
91+
TEST(shell_rejects_redirect_in) {
92+
ASSERT_FALSE(cbm_validate_shell_arg("foo<in.txt"));
93+
PASS();
94+
}
95+
7896
TEST(shell_accepts_clean_path) {
7997
ASSERT_TRUE(cbm_validate_shell_arg("/home/user/.local/bin/codebase-memory-mcp"));
8098
PASS();
@@ -364,6 +382,9 @@ SUITE(security) {
364382
RUN_TEST(shell_rejects_newline);
365383
RUN_TEST(shell_rejects_carriage_return);
366384
RUN_TEST(shell_rejects_null);
385+
RUN_TEST(shell_rejects_double_quote);
386+
RUN_TEST(shell_rejects_redirect_out);
387+
RUN_TEST(shell_rejects_redirect_in);
367388
RUN_TEST(shell_accepts_clean_path);
368389
RUN_TEST(shell_accepts_spaces);
369390
RUN_TEST(shell_accepts_dots_dashes);

0 commit comments

Comments
 (0)