Prefer fstatat + dirfd to cf_strlcat and then stat - #5517
Open
AZero13 wants to merge 1 commit into
Open
Conversation
Prefer fstatat + dirfd to cf_strlcat and then stat
jmschonfeld
reviewed
Jul 22, 2026
jmschonfeld
left a comment
jmschonfeld
left a comment
Contributor
There was a problem hiding this comment.
Could you add a unit test that verifies this functionality to prevent regressions?
| cf_strlcat(subdirPath, dp->d_name, sizeof(subdirPath)); | ||
| if (stat(subdirPath, &statBuf) == 0) { | ||
| #if TARGET_OS_WASI | ||
| // WASI doesn't support dirfd/fstatat, fall back to stat |
Contributor
There was a problem hiding this comment.
@MaxDesiatov just confirming - is this correct?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
When checking if a filesystem entry is a directory (handling DT_UNKNOWN or similar), the existing code relied on copying and concatenating strings into a fixed-size CFMaxPathLength buffer using cf_strlcpy and cf_strlcat. If the combined length of the directory path and filename exceeded the buffer size, cf_strlcat would silently truncate the path. This resulted in passing a corrupted path to stat(), causing it to fail and silently misclassify deeply nested directories as standard files. Furthermore, absolute path construction is generally less efficient and more vulnerable to Time-of-Check to Time-of-Use (TOCTOU) race conditions.
Modifications:
Replaced the manual absolute path construction and stat() call with fstatat() and dirfd() on supported platforms. This allows the kernel to resolve the path relative to the already-open directory descriptor, bypassing buffer limits entirely.
For platforms that lack dirfd/fstatat (like WASI), wrapped the legacy stat() fallback in an explicit bounds check (if (pathLength + 1 + namelen < CFMaxPathLength)) to prevent the silent string truncation bug.
Fixed a pointer arithmetic bug in the file extension matching logic where strchr and wcschr failed to advance past the current dot, causing an infinite loop.