fix: the patch utility derives the target filename f... in inp.c#38
Conversation
Automated security fix generated by Orbis Security AI
There was a problem hiding this comment.
Code Review
This pull request introduces a path traversal check in the scan_input function to validate patch-supplied filenames. The review feedback correctly identifies that the current implementation is incomplete because it does not block absolute paths or Plan 9 specific kernel device paths, which could still allow for system file overwrites. Additionally, a style inconsistency regarding indentation was noted, and a code suggestion was provided to resolve these security and formatting concerns.
| if (strstr(filename, "../") != NULL || strstr(filename, "/..") != NULL | ||
| || strcmp(filename, "..") == 0) | ||
| fatal ("invalid filename `%s' in patch -- potential path traversal", | ||
| filename); |
There was a problem hiding this comment.
The path traversal check is incomplete because it does not block absolute paths (e.g., /etc/passwd), which is explicitly mentioned as a threat in the PR description. An attacker can still overwrite system files by providing an absolute path in the patch header. For Plan 9/APE environments, you should also consider blocking paths starting with # to prevent access to kernel devices. Additionally, the indentation of the fatal call should be updated to use a tab to remain consistent with the existing code style in this file (e.g., line 88).
if (filename[0] == '/' || filename[0] == '#' || strstr(filename, "../") != NULL ||
strstr(filename, "/..") != NULL || strcmp(filename, "..") == 0)
fatal ("invalid filename `%s' in patch -- potential path traversal",
filename);There was a problem hiding this comment.
@orbisai0security can you address code review comments?
|
Something went wrong while applying the changes (e.g. shell or git failed): Reason: Shell execution failed: syntax error at -e line 2, near "filename[" Details:
You can try more specific instructions or apply the change manually. |
|
✅ Changes Applied I've updated the code based on your feedback: The existing path traversal check in
Additionally, the original fix used 4-space indentation inconsistently with the rest of the file which uses tabs. The updated check adds Files modified:
The changes have been pushed to this PR branch. Please review! |
Summary
Fix high severity security issue in
sys/src/ape/cmd/patch/inp.c.Vulnerability
V-007sys/src/ape/cmd/patch/inp.c:58Description: The patch utility derives the target filename from the --- and +++ header lines of a unified diff file and passes it to scan_input() without validating for path traversal sequences. An attacker who can supply a crafted patch file can include filenames such as '../../etc/sudoers' or '/etc/cron.d/backdoor' in the patch headers. When a privileged user or automated system applies the patch, the utility writes attacker-controlled content to arbitrary filesystem locations.
Changes
sys/src/ape/cmd/patch/inp.cVerification
Automated security fix by OrbisAI Security