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 <pengpeng@iscas.ac.cn>

Author: neosys007

compiler/cpp/src/thrift/main.cc

@@ -32,6 +32,7 @@
 #include <stdio.h>
 #include <stdarg.h>
 #include <time.h>
+#include <cstring>
 #include <string>
 #include <algorithm>
 #include <sys/types.h>
@@ -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);