Pengpeng Hou created THRIFT-5932:
------------------------------------
Summary: saferealpath() on Windows can strcpy() an arbitrarily
long path into caller storage
Key: THRIFT-5932
URL: https://issues.apache.org/jira/browse/THRIFT-5932
Project: Thrift
Issue Type: Bug
Reporter: Pengpeng Hou
Hi,
I rechecked current upstream head and this Windows fallback still looks unsafe.
In `compiler/cpp/src/thrift/main.cc`, `saferealpath()` uses
`GetFullPathNameA()` on Windows and then falls back to copying the original
path into `resolved_path` with:
if (len == 0 || len > MAX_PATH - 1) {
strcpy(resolved_path, path);
} else {
strcpy(resolved_path, buf);
}
The fallback does not know the actual size of `resolved_path`, and the callers
typically pass a fixed stack buffer such as `THRIFT_PATH_MAX` in the compiler
front end. If the input `.thrift` path is too long, or if `GetFullPathNameA()`
fails, this code copies the raw path into the caller buffer without any local
bound check.
I rechecked the current code and did not find any capacity-aware copy or
rejection of overlong paths before the `strcpy()`.
*How to reproduce*
1. Run the Thrift C++ compiler on Windows.
2. Pass a very long `.thrift` file path so that `GetFullPathNameA()` returns 0
or a length greater than `MAX_PATH - 1`.
3. The fallback path calls `strcpy(resolved_path, path)`.
4. If the caller's `resolved_path` buffer is the usual fixed stack buffer, the
copy can overflow it.
This is easiest to observe with ASan or a debugger, but the bug is in the
fallback logic itself.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
