diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index c3033ce..9664767 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -70,11 +70,100 @@ static void set_reason(char *buf, size_t cap, const char *prefix, const char *de buf[cap - 1] = '\0'; } -/** Return 1 if @p s begins with prefix after any leading whitespace. */ +/** + * Strip one matching pair of surrounding quotes so `"$HOME/x"` is checked + * the same way as `$HOME/x`. /bin/sh still expands the inner form. + */ +static char *strip_surrounding_quotes(char *tok) +{ + size_t n; + if (!tok || !tok[0]) return tok; + n = strlen(tok); + if (n >= 2 && ((tok[0] == '\'' && tok[n - 1] == '\'') || + (tok[0] == '"' && tok[n - 1] == '"'))) { + tok[n - 1] = '\0'; + return tok + 1; + } + return tok; +} + +/** + * Return 1 if @p tok looks like a filesystem path (or expands to one). + * Includes shell parameter expansions such as `$HOME/...` and `${PWD}/...` + * that never start with `/` `~` `.` but become absolute after /bin/sh expands them. + */ static int has_path_chars(const char *tok) { - if (!tok) return 0; - return tok[0] == '/' || tok[0] == '~' || tok[0] == '.'; + if (!tok || !tok[0]) return 0; + if (tok[0] == '/' || tok[0] == '~' || tok[0] == '.') return 1; + /* `$HOME`, `${HOME}/x`, `$PWD/../out`, `$(pwd)/x`, ANSI-C `$'\x2f...'`. */ + if (tok[0] == '$') return 1; + return 0; +} + +/** + * Expand a leading `~`, `$HOME` / `${HOME}`, or `$PWD` / `${PWD}` for workspace checks. + * Unknown `$...` forms fail closed (return -1) so workspace_only cannot be bypassed + * via arbitrary parameter / command substitutions. + * @return 0 and writes into @p expanded on success; -1 if the token must be blocked. + */ +static int expand_shell_path_token(const char *tok, char *expanded, size_t expanded_cap) +{ + const char *home; + const char *cwd; + const char *suffix; + int n; + + if (!tok || !expanded || expanded_cap == 0) return -1; + + if (tok[0] == '~') { + home = getenv("HOME"); + if (home) + n = snprintf(expanded, expanded_cap, "%s%s", home, tok + 1); + else + n = snprintf(expanded, expanded_cap, "%s", tok); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + + if (tok[0] != '$') { + if (strlen(tok) >= expanded_cap) return -1; + memcpy(expanded, tok, strlen(tok) + 1); + return 0; + } + + /* Fail closed on ANSI-C quoting and command substitution (encoded / dynamic paths). */ + if (tok[1] == '\'' || tok[1] == '"' || tok[1] == '(') + return -1; + + home = getenv("HOME"); + cwd = getenv("PWD"); + if (strncmp(tok, "${HOME}", 7) == 0) { + suffix = tok + 7; + if (!home) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", home, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "$HOME", 5) == 0 && (tok[5] == '\0' || tok[5] == '/')) { + suffix = tok + 5; + if (!home) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", home, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "${PWD}", 6) == 0) { + suffix = tok + 6; + if (!cwd) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", cwd, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + if (strncmp(tok, "$PWD", 4) == 0 && (tok[4] == '\0' || tok[4] == '/')) { + suffix = tok + 4; + if (!cwd) return -1; + n = snprintf(expanded, expanded_cap, "%s%s", cwd, suffix); + return (n < 0 || (size_t)n >= expanded_cap) ? -1 : 0; + } + + /* Other `$VAR` / `${VAR}` forms: fail closed under workspace_only. */ + return -1; } /* ------------------------------------------------------------------ */ @@ -166,21 +255,22 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg if (!cmd_copy) return 0; /* fail-open on OOM */ tok = strtok_r(cmd_copy, " \t\n;|&><", &saveptr); while (tok) { + tok = strip_surrounding_quotes(tok); if (has_path_chars(tok)) { - /* Expand a leading tilde naively */ char expanded[PATH_MAX]; - if (tok[0] == '~') { - const char *home = getenv("HOME"); - if (home) - snprintf(expanded, sizeof(expanded), "%s%s", home, tok + 1); - else - snprintf(expanded, sizeof(expanded), "%s", tok); - tok = expanded; + const char *check_path = tok; + if (expand_shell_path_token(tok, expanded, sizeof(expanded)) != 0) { + set_reason(reason_buf, reason_cap, + "command blocked: unresolved shell path expansion: ", tok); + fprintf(stderr, "allowlist: blocked unresolved shell path: %s\n", tok); + free(cmd_copy); + return 1; } - if (!allowlist_path_is_under_workspace(tok, workspace_root)) { + check_path = expanded; + if (!allowlist_path_is_under_workspace(check_path, workspace_root)) { set_reason(reason_buf, reason_cap, - "command blocked: path escapes workspace: ", tok); - fprintf(stderr, "allowlist: blocked path outside workspace: %s\n", tok); + "command blocked: path escapes workspace: ", check_path); + fprintf(stderr, "allowlist: blocked path outside workspace: %s\n", check_path); free(cmd_copy); return 1; } diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 76a4e4f..5bc7da0 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -9,7 +9,9 @@ * "mkfs", "dd of=/dev/", fork bombs, etc.). * 2. An optional workspace-containment check: if enabled via allowlist_config_t, * path-like tokens in the command are resolved with realpath(3) and rejected - * when they escape the declared workspace root. + * when they escape the declared workspace root. Tokens that become absolute only + * after shell expansion (`$HOME/...`, `"$HOME/..."`, `${PWD}/...`, ANSI-C + * `$'\x2f...'`) are unquoted, expanded, or fail-closed before that check. * * Both checks are intentionally conservative and may produce false positives. * They are a best-effort defence-in-depth layer; real isolation is provided by diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index 5d2ac74..6ea9131 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -144,6 +144,54 @@ static int test_workspace_only_allows_inside_path(void) return 0; } +/** + * Shell expands `$HOME` / `${HOME}` / `$PWD` before open(2). Tokens never start + * with `/` `~` `.`, so the old has_path_chars gate skipped them and workspace_only + * could not stop `cat $HOME/.shellclaw/auth_tokens.json`. + */ +static int test_workspace_only_blocks_home_env_expansion(void) +{ + allowlist_config_t cfg; + char reason[256]; + char ws[] = "/tmp/sc_al_home_XXXXXX"; + char *dir; + const char *home = getenv("HOME"); + + dir = mkdtemp(ws); + if (!dir) { + fprintf(stderr, "test_workspace_only_blocks_home_env_expansion: mkdtemp failed\n"); + return 1; + } + cfg.workspace_path = dir; + cfg.workspace_only = 1; + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $HOME/.shellclaw/auth_tokens.json", + &cfg, reason, sizeof(reason)) == 1); + ASSERT(strstr(reason, "escapes workspace") != NULL || + strstr(reason, "unresolved") != NULL); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat ${HOME}/.shellclaw/auth_tokens.json", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $PWD/../outside.txt", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat $'\\x2fetc\\x2fpasswd'", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat \"$HOME/.shellclaw/auth_tokens.json\"", + &cfg, reason, sizeof(reason)) == 1); + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("cat notes.txt", &cfg, reason, sizeof(reason)) == 0); + if (home && strcmp(home, dir) == 0) { + /* Degenerate: HOME equals workspace — $HOME alone is in-bounds. */ + reason[0] = '\0'; + ASSERT(allowlist_check_shell_command("ls $HOME", &cfg, reason, sizeof(reason)) == 0); + } + rmdir(dir); + return 0; +} + /* ------------------------------------------------------------------ */ /* Symlink escape test (5.4) */ /* ------------------------------------------------------------------ */ @@ -205,6 +253,7 @@ int main(void) RUN(test_path_prefix_no_slash()); RUN(test_workspace_only_blocks_outside_path()); RUN(test_workspace_only_allows_inside_path()); + RUN(test_workspace_only_blocks_home_env_expansion()); RUN(test_symlink_escape()); printf("test_allowlist: all tests passed\n"); return 0;