Conversation
| const char *in = argv[1]; | ||
| const char *out = argv[2]; | ||
|
|
||
| char *src = read_file(in); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
In general, to fix uncontrolled path usage from command-line arguments, you must validate or normalize the user-supplied path before using it in any file access function. For a command-line tool like this builder, a reasonable policy is to only accept non-empty relative paths that do not escape their starting directory (that is, they contain no ".." components) and optionally to disallow ambiguous components such as ".".
In this specific file, the overall structure is already correct: both in and out are validated via is_safe_relative_path before being used with read_file and write_spc. The best minimal fix that strengthens the defense, without changing the public behavior for “normal” use, is to slightly tighten is_safe_relative_path to also reject "." as a path component. This avoids accepting inputs such as ".", "./", or similar, which don't represent a real file and could be confusing or misused. Everything else in main can stay as is because the validation is already applied before file operations.
Concretely:
- Edit
src/build.c, in the definition ofis_safe_relative_path. - Inside the loop that checks path components, change the condition that currently rejects only
".."to reject both".."and".". - No new functions or imports are required; we only adjust the logic inside the existing helper.
This keeps the validation localized and clear, and slightly more conservative, making it easier for both humans and tools to see that user-controlled paths are being constrained before use.
| @@ -780,7 +780,7 @@ | ||
| return 0; | ||
| } | ||
|
|
||
| /* Scan components separated by '/' or '\\' and reject any that are exactly "..". */ | ||
| /* Scan components separated by '/' or '\\' and reject any that are exactly "." or "..". */ | ||
| const char *p = path; | ||
| while (*p) { | ||
| while (*p == '/' || *p == '\\') { | ||
| @@ -794,7 +794,10 @@ | ||
| p++; | ||
| } | ||
| size_t len = (size_t)(p - start); | ||
| if (len == 2 && start[0] == '.' && start[1] == '.') { | ||
| /* Reject single-dot and double-dot components to avoid ambiguous or | ||
| traversing path segments like ".", "./", "../", etc. */ | ||
| if ((len == 1 && start[0] == '.') || | ||
| (len == 2 && start[0] == '.' && start[1] == '.')) { | ||
| return 0; | ||
| } | ||
| } |
|
|
||
| if (!write_ast_to_spc(argv[2], root)) { | ||
| fprintf(stderr, "Could not write: %s\n", argv[2]); | ||
| if (!write_spc(out, root)) { |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 7 days ago
In general terms, the fix is to ensure that the user-supplied out path cannot escape an intended safe directory and cannot use traversal constructs or absolute paths. This is typically done by constructing a full path from a known base directory and the user input, resolving it (e.g., with realpath), and checking that the resolved path remains within the base directory before passing it to any file-opening function. Since we must not change functionality more than necessary and can only edit src/build.c, the best approach is to enhance and reuse the existing is_safe_relative_path logic and add a small helper in main to canonicalize and re-check the output path before calling write_spc.
Concretely, we can:
- Keep
is_safe_relative_pathas a first, cheap filter on bothinandout. - For the output path, derive an absolute, normalized path with
realpathbefore passing it towrite_spc.- Because the file might not exist yet, we cannot directly call
realpath(out, ...)successfully in all cases. Instead, we can:- Resolve the current working directory with
getcwd, and - Join it with
outto build a tentative full path, then callrealpathon the containing directory and reconstruct the final path, or - More simply, we can rely on
is_safe_relative_pathto ensureoutis purely relative (no..), and then prepend a chosen safe base directory such as the current working directory at runtime and pass that joined path towrite_spc. This ensures that even ifwrite_spcopens paths directly, the actual file is created under this base directory.
- Resolve the current working directory with
- Because the file might not exist yet, we cannot directly call
- Introduce a small helper, for example
static char *build_output_path(const char *rel_out), that:- Assumes
rel_outhas already passedis_safe_relative_path. - Gets the current working directory via
getcwd. - Allocates a buffer large enough for
cwd + "/" + rel_out. - Concatenates them to form an absolute path and returns it (caller frees).
- Assumes
- In
main, replace the directwrite_spc(out, root)call with a call tobuild_output_path(out)and then pass the resulting safe absolute path intowrite_spc. If building the path fails, we report an error and exit.
This preserves existing semantics in a typical use case (writing under the current directory) while hardening the path handling and making the data passed into write_spc clearly controlled. It also introduces no new external dependencies beyond standard POSIX APIs already in use (getcwd from <unistd.h>).
| Read SPC into memory | ||
| ========================= */ | ||
|
|
||
| FILE *f = fopen(path, "rb"); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
|
Something is broken |
Check the code