Fix TOCTOU race condition in file content probes - #2406
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| goto cleanup; | ||
|
|
||
| fd = open(whole_path_with_prefix, O_RDONLY); | ||
| fd = open(whole_path_with_prefix, O_RDONLY | O_NONBLOCK); |
| */ | ||
| whole_path_with_prefix = oscap_path_join(prefix, whole_path); | ||
| if (stat(whole_path_with_prefix, &st) == -1) | ||
| tmp_fd = open(whole_path_with_prefix, O_RDONLY | O_NONBLOCK); |
There was a problem hiding this comment.
@jan-cerny, @Mab879, are these warnings from github-bot legit?
There was a problem hiding this comment.
I think they are legit. Any users can set the OSCAP_PROBE_ROOT environment variable to any string and this string will be used as a prefix prepended to the file path, which can potentionally lead to opening arbitrary or unwanted files in the open call. However, this is a pre-existing issue, and we haven't cared so far.
| } | ||
| if (fstat(tmp_fd, &st) == -1 | ||
| || !S_ISREG(st.st_mode) | ||
| || probe_fd_path_is_blocked(tmp_fd, ctx->blocked_paths)) |
There was a problem hiding this comment.
IIUC if the prefix is used the probe_fd_path_is_blocked will check the path including the prefix, but we want to check the path without the prefix.
There was a problem hiding this comment.
Correct, because blocked_paths are not prefixed. Updated
9bb95c6 to
147e453
Compare
|
3 participants
Description
textfilecontent54textfilecontentyamlfilecontentRationale
stat()followed byopen()in mentioned probes creates a tight vulnerability window, which attacker may use to switch paths during the file processingopen()first we getting a file descriptor early, which prevents race conditionfstat()to confirm the presence of a valid scanning targetO_NONBLOCKflag toopen()calls to prevent FIFO-based denial-of-service (a named pipe without a writer would block the scanner indefinitely)fopen()withfdopen()intextfilecontent_probe.candyamlfilecontent_probe.cto wrap the already-opened fd instead of performing a second path resolutionS_ISREG()check toyamlfilecontent_probe.c, which previously had none - it would attempt to parse directories, sockets, or other non-regular files as YAMLprobe_fd_path_is_blocked()- a new helper inprobe-api.cthat resolves an open fd's real path via/proc/self/fd/<fd>and checks it against the blocked paths listTesting
ctest --test-dir build -R "textfilecontent|yamlfilecontent" -j$(nproc)probes/textfilecontent54/test_symlinks.shpassed