Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| if (!write_ast_to_spc(argv[2], root)) { | ||
| fprintf(stderr, "Could not write: %s\n", argv[2]); | ||
| if (!write_spc(out_path, root)) { |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
Copilot Autofix
AI 8 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| Read SPC into memory | ||
| ========================= */ | ||
|
|
||
| FILE *f = fopen(path, "rb"); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
In general, the fix is to avoid passing raw user input into fopen and instead (1) validate that the input is syntactically safe (no absolute paths, no .. components, etc.) and (2) ensure that the resolved path is contained within an allowed base directory. The second step typically uses realpath (or _fullpath on Windows) to normalize the user-specified path and compare it to a canonical base directory; if the resolved path is not under the base directory, the input is rejected.
For this specific file, we already have is_safe_relative_path doing basic syntactic validation. The best way to harden the code without changing intended functionality is:
- Determine a base directory. Since we have
splice_getcwddefined asgetcwd/_getcwd, we can use the process’s current working directory as the base. - Canonicalize that base directory using
realpath(or_fullpathon Windows) to avoid issues with symlinks and relative components. - Canonicalize the user-supplied path (which is already checked to be relative) into an absolute path relative to the current working directory, again using
realpath/_fullpath. - Ensure that the canonicalized file path starts with the canonical base directory plus a path separator. If not, reject the input.
- Use the validated, canonical path when calling
fopeninstead of the rawargv[1].
We can implement this entirely inside main in src/splice.c, right after verifying the extension and before calling fopen. We will:
- Add a small block to obtain and canonicalize the base directory.
- Add another block to canonicalize
pathand check that it lies under the base directory. - Replace
fopen(path, "rb")withfopen(resolved_path, "rb").
We already include<limits.h>and<unistd.h>, andPATH_MAXis defined; we do not need new headers. We will userealpathdirectly, which is widely available on POSIX; for Windows builds, this project already uses_getcwd, andrealpathis commonly available via POSIX-compatibility layers, but since we cannot modify unseen platform abstractions, we will still callrealpathguarded by standard behavior and handle failure gracefully.
| @@ -103,13 +103,55 @@ | ||
| return 1; | ||
| } | ||
|
|
||
| /* Resolve and constrain path to the current working directory to | ||
| mitigate path traversal and unintended file access. */ | ||
| char cwd_buf[PATH_MAX]; | ||
| if (splice_getcwd(cwd_buf, sizeof(cwd_buf)) == NULL) { | ||
| perror("getcwd"); | ||
| return 1; | ||
| } | ||
|
|
||
| char *base_real = realpath(cwd_buf, NULL); | ||
| if (base_real == NULL) { | ||
| perror("realpath"); | ||
| return 1; | ||
| } | ||
|
|
||
| char path_buf[PATH_MAX]; | ||
| /* Build a path relative to the current working directory. */ | ||
| if (snprintf(path_buf, sizeof(path_buf), "%s/%s", cwd_buf, path) >= (int)sizeof(path_buf)) { | ||
| fprintf(stderr, "[ERROR] SPC path too long\n"); | ||
| free(base_real); | ||
| return 1; | ||
| } | ||
|
|
||
| char *resolved_path = realpath(path_buf, NULL); | ||
| if (resolved_path == NULL) { | ||
| perror("realpath"); | ||
| free(base_real); | ||
| return 1; | ||
| } | ||
|
|
||
| size_t base_len = strlen(base_real); | ||
| /* Ensure the resolved file path resides within the base directory. */ | ||
| if (strncmp(base_real, resolved_path, base_len) != 0 || | ||
| (resolved_path[base_len] != '/' && resolved_path[base_len] != '\0')) { | ||
| fprintf(stderr, "[ERROR] SPC path escapes working directory\n"); | ||
| free(base_real); | ||
| free(resolved_path); | ||
| return 1; | ||
| } | ||
|
|
||
| free(base_real); | ||
|
|
||
| /* ========================= | ||
| Read SPC into memory | ||
| ========================= */ | ||
|
|
||
| FILE *f = fopen(path, "rb"); | ||
| FILE *f = fopen(resolved_path, "rb"); | ||
| if (!f) { | ||
| perror("fopen"); | ||
| free(resolved_path); | ||
| return 1; | ||
| } | ||
|
|
|
send another pull and i will review |
1 participant
No description provided.