[
https://issues.apache.org/jira/browse/THRIFT-5932?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sylwester Lachiewicz resolved THRIFT-5932.
------------------------------------------
Fix Version/s: 0.23.0
Resolution: Fixed
Resolving. {{saferealpath()}} in {{compiler/cpp/src/thrift/main.cc}} now takes
a {{resolved_path_size}} parameter, and both call sites pass
{{THRIFT_PATH_MAX}}, so the output is bounded by the caller's buffer. Commit
{{0be731cc3}}, first released in 0.23.0.
_This comment was created with AI assistance._
> 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
> Components: Compiler (General)
> Reporter: Pengpeng Hou
> Priority: Major
> Fix For: 0.23.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> 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 message was sent by Atlassian Jira
(v8.20.10#820010)
