[jira] [Resolved] (THRIFT-5932) saferealpath() on Windows can strcpy() an arbitrarily long path into caller storage

["Sylwester Lachiewicz (Jira)" <ji...@apache.org>]

     [ 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)