fix(fileutils): prevent stack buffer overflow in AbsPath#16
Open
Rakdos8 wants to merge 1 commit into
Open
Conversation
getcwd + path concatenation and the absolute-path strcpy wrote into fixed PATH_MAX buffers with no bounds check, allowing a stack overflow for long paths. Build the working buffer with bounded snprintf, bail out safely (empty result) when the path cannot fit, and pass the output buffer size explicitly. Add regression tests for overlong relative and absolute paths.
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
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.
Problem
AbsPath(POSIX path, used byCcpGetAbsolutePath) wrote intochar[PATH_MAX]buffers with no bounds checking.getcwd(src, …)followed bystrcat(src, path)overflows the stack whencwd + pathexceedsPATH_MAX(1024 onmacOS);
src[len] = '/'; src[len+1] = 0;writes out of bounds whenlen == PATH_MAX-1; andstrcpy(src, path)(absolute path) plus
strcpy(realPath, path)(ongetcwdfailure) are unbounded too. This is reachable on macOS (aprimary CI target) via
CcpGetAbsolutePath, so it is a high-severity stack overflow driven entirely by path length.Fix
AbsPathnow receives the output buffer size (size_t realPathSize) — there is a single internal caller.srcisbuilt with a bounded
snprintfand the function returns an empty string when the combined path does not fit. Thegetcwdfailure path usesstrncpy_s(…, _TRUNCATE)instead ofstrcpy, and there is an explicit guard beforewriting into
realPath.CcpGetAbsolutePathdrops a dead line (astrcpy_sthat was immediately overwritten) andits unused variable, and now passes
sizeof(buffer). Added#include <cstdio>in the POSIX block. Contract: when apath is too long to fit, the function returns an empty string instead of overflowing.
Tests
Added two regression tests in
tests/CcpFileUtils.cpp(GoogleTest,#if !_WIN32):CcpGetAbsolutePathHandlesOverlongRelativePathSafelyandCcpGetAbsolutePathHandlesOverlongAbsolutePathSafely, bothusing 64 KB paths and expecting an empty result with no crash. Existing path-resolution tests are unaffected. Reviewed
manually; not compiled locally as the vcpkg toolchain is unavailable in this environment.
Scope
POSIX branch only — the Windows path uses
GetFullPathNameWand is unchanged. No public API change.