From 57b5f1c9867476a1f56ea31b1d4967f0196b7711 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 14 Aug 2026 11:37:14 +0000 Subject: [PATCH] fix(sandbox): block .. escapes when shell destinations do not exist Lexical prefix matching treated workspace/../../tmp/newfile as inside the workspace whenever realpath failed. sandbox_exec does not pivot_root, so that was a real filesystem escape. Walk to the first existing ancestor instead, matching tools/file.c. Co-authored-by: esadrianno --- src/sandbox/allowlist.c | 53 ++++++++++++++++++++++++++++------------- src/sandbox/allowlist.h | 8 +++++-- tests/test_allowlist.c | 43 +++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 19 deletions(-) diff --git a/src/sandbox/allowlist.c b/src/sandbox/allowlist.c index c3033ce..4fc5a11 100644 --- a/src/sandbox/allowlist.c +++ b/src/sandbox/allowlist.c @@ -7,6 +7,7 @@ #include "sandbox/allowlist.h" #include +#include #include #include #include @@ -77,6 +78,38 @@ static int has_path_chars(const char *tok) return tok[0] == '/' || tok[0] == '~' || tok[0] == '.'; } +static int resolved_is_under_workspace(const char *resolved, const char *actual_ws, size_t wlen) +{ + if (!resolved || !actual_ws || wlen == 0) return 0; + if (strncmp(resolved, actual_ws, wlen) != 0) return 0; + return resolved[wlen] == '\0' || resolved[wlen] == '/'; +} + +/* + * realpath(3) cannot canonicalize a path that does not exist. Walking to the + * first existing ancestor (same approach as tools/file.c) still collapses `..` + * through existing directories, so workspace/../../tmp/newfile is denied. + * A lexical prefix check would allow that destination. + */ +static int existing_ancestor_is_under_workspace(const char *path, const char *actual_ws, size_t wlen) +{ + char path_copy[PATH_MAX]; + char resolved[PATH_MAX]; + int hops; + if (!path || path[0] == '\0' || strlen(path) >= PATH_MAX) return 0; + snprintf(path_copy, sizeof(path_copy), "%s", path); + for (hops = 0; hops < PATH_MAX; hops++) { + char *dir = dirname(path_copy); + if (!dir || dir[0] == '\0') return 0; + if (realpath(dir, resolved) != NULL) + return resolved_is_under_workspace(resolved, actual_ws, wlen); + if (strcmp(dir, ".") == 0 || strcmp(dir, "/") == 0) return 0; + if (dir != path_copy) + snprintf(path_copy, sizeof(path_copy), "%s", dir); + } + return 0; +} + /* ------------------------------------------------------------------ */ /* Public: path-under-workspace check (5.4) */ /* ------------------------------------------------------------------ */ @@ -94,23 +127,9 @@ int allowlist_path_is_under_workspace(const char *path, const char *workspace_ro else actual_ws = workspace_root; wlen = strlen(actual_ws); - if (realpath(path, resolved_path)) { - /* Exact match or resolved path starts with resolved workspace + '/' */ - if (strncmp(resolved_path, actual_ws, wlen) == 0) { - if (resolved_path[wlen] == '\0' || resolved_path[wlen] == '/') return 1; - } - return 0; - } - /* Path does not exist on disk: check the lexical prefix against resolved workspace. */ - if (strncmp(path, actual_ws, wlen) == 0) { - if (path[wlen] == '\0' || path[wlen] == '/') return 1; - } - /* Also try against the original (unresolved) workspace root. */ - wlen = strlen(workspace_root); - if (strncmp(path, workspace_root, wlen) == 0) { - if (path[wlen] == '\0' || path[wlen] == '/') return 1; - } - return 0; + if (realpath(path, resolved_path)) + return resolved_is_under_workspace(resolved_path, actual_ws, wlen); + return existing_ancestor_is_under_workspace(path, actual_ws, wlen); } /* ------------------------------------------------------------------ */ diff --git a/src/sandbox/allowlist.h b/src/sandbox/allowlist.h index 76a4e4f..23d41be 100644 --- a/src/sandbox/allowlist.h +++ b/src/sandbox/allowlist.h @@ -57,8 +57,12 @@ int allowlist_check_shell_command(const char *cmd, const allowlist_config_t *cfg /** * Check whether @p path is contained inside @p workspace_root after resolving symlinks. * - * Uses realpath(3); if the path does not exist on disk, checks the string prefix - * against the canonicalised workspace root. + * Uses realpath(3) when the path exists. If it does not, walks to the first + * existing ancestor and checks that resolved directory (so `..` cannot escape + * by targeting a file that has not been created yet). + * + * Example: allowlist_path_is_under_workspace("/ws/../../tmp/x", "/ws") is 0 + * even when /tmp/x does not exist. * * @param path Absolute or relative path to test. * @param workspace_root Absolute path to the workspace root (already resolved). diff --git a/tests/test_allowlist.c b/tests/test_allowlist.c index 5d2ac74..f2c0a2a 100644 --- a/tests/test_allowlist.c +++ b/tests/test_allowlist.c @@ -184,6 +184,48 @@ static int test_symlink_escape(void) #endif } +/* ------------------------------------------------------------------ */ +/* Non-existent path with .. must not escape via lexical prefix */ +/* ------------------------------------------------------------------ */ + +static int test_dotdot_escape_nonexistent_destination(void) +{ + char workspace[] = "/tmp/sc_al_ws_XXXXXX"; + char *ws; + char new_file[256]; + char escape_path[256]; + char cmd[640]; + allowlist_config_t cfg; + char reason[256]; + + ws = mkdtemp(workspace); + if (!ws) { + fprintf(stderr, "test_dotdot_escape_nonexistent_destination: mkdtemp failed\n"); + return 1; + } + + /* realpath() fails for a new file; the workspace ancestor must still allow it. */ + snprintf(new_file, sizeof(new_file), "%s/brand_new.txt", ws); + ASSERT(allowlist_path_is_under_workspace(new_file, ws) == 1); + + /* + * Destination does not exist, so realpath() fails. A lexical prefix check + * treats workspace/../../tmp/... as inside the workspace. + */ + snprintf(escape_path, sizeof(escape_path), + "%s/../../tmp/sc_al_stolen_%d", ws, (int)getpid()); + ASSERT(allowlist_path_is_under_workspace(escape_path, ws) == 0); + + cfg.workspace_path = ws; + cfg.workspace_only = 1; + reason[0] = '\0'; + snprintf(cmd, sizeof(cmd), "cp %s/memory.db %s", ws, escape_path); + ASSERT(allowlist_check_shell_command(cmd, &cfg, reason, sizeof(reason)) == 1); + + rmdir(ws); + return 0; +} + /* ------------------------------------------------------------------ */ /* main */ /* ------------------------------------------------------------------ */ @@ -206,6 +248,7 @@ int main(void) RUN(test_workspace_only_blocks_outside_path()); RUN(test_workspace_only_allows_inside_path()); RUN(test_symlink_escape()); + RUN(test_dotdot_escape_nonexistent_destination()); printf("test_allowlist: all tests passed\n"); return 0; }