From 72e169cedb51b4ef7595de9910c5904722356927 Mon Sep 17 00:00:00 2001 From: Pengpeng Hou Date: Sun, 22 Mar 2026 07:26:01 +0800 Subject: [PATCH 1/2] THRIFT-5932: compiler/cpp: bound saferealpath() output on Windows The Windows fallback in saferealpath() still copies either the original path or the GetFullPathNameA() result into resolved_path with strcpy(), but the helper does not know the caller buffer size. Make saferealpath() size-aware, reject paths that do not fit the caller buffer, and keep the existing call sites using their fixed THRIFT_PATH_MAX storage. Signed-off-by: Pengpeng Hou --- compiler/cpp/src/thrift/main.cc | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/compiler/cpp/src/thrift/main.cc b/compiler/cpp/src/thrift/main.cc index dfd22e9a0da..d1345f2e55d 100644 --- a/compiler/cpp/src/thrift/main.cc +++ b/compiler/cpp/src/thrift/main.cc @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -164,27 +165,35 @@ bool g_generator_failure = false; * Win32 doesn't have realpath, so use fallback implementation in that case, * otherwise this just calls through to realpath */ -char* saferealpath(const char* path, char* resolved_path) { +char* saferealpath(const char* path, char* resolved_path, size_t resolved_path_size) { #ifdef _WIN32 char buf[MAX_PATH]; char* basename; + const char* source; + size_t source_len; DWORD len = GetFullPathNameA(path, MAX_PATH, buf, &basename); if (len == 0 || len > MAX_PATH - 1) { - strcpy(resolved_path, path); + source = path; } else { - strcpy(resolved_path, buf); + source = buf; } + source_len = strlen(source); + if (source_len >= resolved_path_size) { + return nullptr; + } + memcpy(resolved_path, source, source_len + 1); + // Replace backslashes with forward slashes so the // rest of the code behaves correctly. - size_t resolved_len = strlen(resolved_path); - for (size_t i = 0; i < resolved_len; i++) { + for (size_t i = 0; i < source_len; i++) { if (resolved_path[i] == '\\') { resolved_path[i] = '/'; } } return resolved_path; #else + (void) resolved_path_size; return realpath(path, resolved_path); #endif } @@ -337,7 +346,7 @@ string include_file(string filename) { // Realpath! char rp[THRIFT_PATH_MAX]; // cppcheck-suppress uninitvar - if (saferealpath(filename.c_str(), rp) == nullptr) { + if (saferealpath(filename.c_str(), rp, THRIFT_PATH_MAX) == nullptr) { pwarning(0, "Cannot open include file %s\n", filename.c_str()); return std::string(); } @@ -360,7 +369,7 @@ string include_file(string filename) { // Realpath! char rp[THRIFT_PATH_MAX]; // cppcheck-suppress uninitvar - if (saferealpath(sfilename.c_str(), rp) == nullptr) { + if (saferealpath(sfilename.c_str(), rp, THRIFT_PATH_MAX) == nullptr) { continue; } @@ -1174,7 +1183,7 @@ int main(int argc, char** argv) { char old_thrift_file_rp[THRIFT_PATH_MAX]; // cppcheck-suppress uninitvar - if (saferealpath(arg, old_thrift_file_rp) == nullptr) { + if (saferealpath(arg, old_thrift_file_rp, THRIFT_PATH_MAX) == nullptr) { failure("Could not open input file with realpath: %s", arg); } old_input_file = string(old_thrift_file_rp); @@ -1232,7 +1241,7 @@ int main(int argc, char** argv) { usage(); } // cppcheck-suppress uninitvar - if (saferealpath(argv[i], new_thrift_file_rp) == nullptr) { + if (saferealpath(argv[i], new_thrift_file_rp, THRIFT_PATH_MAX) == nullptr) { failure("Could not open input file with realpath: %s", argv[i]); } string new_input_file(new_thrift_file_rp); @@ -1258,7 +1267,7 @@ int main(int argc, char** argv) { usage(); } // cppcheck-suppress uninitvar - if (saferealpath(argv[i], rp) == nullptr) { + if (saferealpath(argv[i], rp, THRIFT_PATH_MAX) == nullptr) { failure("Could not open input file with realpath: %s", argv[i]); } string input_file(rp); From 6e59e508042f65975ce4a04751b97e3c69c0cfed Mon Sep 17 00:00:00 2001 From: Pengpeng Hou Date: Wed, 25 Mar 2026 09:01:45 +0800 Subject: [PATCH 2/2] compiler: use strcpy() for checked saferealpath() copy Use strcpy() once the source path has been checked to fit in the\ndestination buffer. This keeps the operation expressed as a string copy,\nwhich is clearer in this code path. --- compiler/cpp/src/thrift/main.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/cpp/src/thrift/main.cc b/compiler/cpp/src/thrift/main.cc index d1345f2e55d..38d764dad61 100644 --- a/compiler/cpp/src/thrift/main.cc +++ b/compiler/cpp/src/thrift/main.cc @@ -182,7 +182,7 @@ char* saferealpath(const char* path, char* resolved_path, size_t resolved_path_s if (source_len >= resolved_path_size) { return nullptr; } - memcpy(resolved_path, source, source_len + 1); + strcpy(resolved_path, source); // Replace backslashes with forward slashes so the // rest of the code behaves correctly.