From 1f531fb62543bf5c69410317c98b2af93a53c1e4 Mon Sep 17 00:00:00 2001
From: Even Rouault <even.rouault@spatialys.com>
Date: Sat, 11 Jan 2025 14:04:09 +0100
Subject: [PATCH] Fix read heap-buffer-overflow on nested /vsitar/ calls

Fixes https://issues.oss-fuzz.com/issues/388868487

This is just a band-aid... The use of CPLFormFilename() and
friends is so dangerous with the rotating TLS buffers that
may end up overwritten if too many calls are nested...
---
 port/cpl_path.cpp                  | 10 ++++++++--
 port/cpl_vsil_abstract_archive.cpp | 31 +++++++++++++++++++++++-------
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/port/cpl_path.cpp b/port/cpl_path.cpp
index 568daff37a0a..2f5642e1c71d 100644
--- a/port/cpl_path.cpp
+++ b/port/cpl_path.cpp
@@ -801,8 +801,11 @@ const char *CPLFormCIFilename(const char *pszPath, const char *pszBasename,
             pszFilename[i] = static_cast<char>(CPLToupper(pszFilename[i]));
         }
 
-        pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
+        const std::string osTmpPath(
+            CPLFormFilename(pszPath, pszFilename, nullptr));
         nStatRet = VSIStatExL(pszFullPath, &sStatBuf, VSI_STAT_EXISTS_FLAG);
+        if (nStatRet == 0)
+            pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
     }
 
     if (nStatRet != 0)
@@ -813,8 +816,11 @@ const char *CPLFormCIFilename(const char *pszPath, const char *pszBasename,
                 CPLTolower(static_cast<unsigned char>(pszFilename[i])));
         }
 
-        pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
+        const std::string osTmpPath(
+            CPLFormFilename(pszPath, pszFilename, nullptr));
         nStatRet = VSIStatExL(pszFullPath, &sStatBuf, VSI_STAT_EXISTS_FLAG);
+        if (nStatRet == 0)
+            pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
     }
 
     if (nStatRet != 0)
diff --git a/port/cpl_vsil_abstract_archive.cpp b/port/cpl_vsil_abstract_archive.cpp
index 2a6c77cec89d..5b1e9a577574 100644
--- a/port/cpl_vsil_abstract_archive.cpp
+++ b/port/cpl_vsil_abstract_archive.cpp
@@ -452,7 +452,12 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
 
     const std::vector<CPLString> oExtensions = GetExtensions();
     int nAttempts = 0;
-    while (pszFilename[i])
+    // If we are called with pszFilename being one of the TLS buffers returned
+    // by cpl_path.cpp functions, then a call to Stat() in the loop (and its
+    // cascaded calls to other cpl_path.cpp functions) might lead to altering
+    // the buffer to be altered, hence take a copy
+    const std::string osFilenameCopy(pszFilename);
+    while (i < static_cast<int>(osFilenameCopy.size()))
     {
         int nToSkip = 0;
 
@@ -460,7 +465,7 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
              iter != oExtensions.end(); ++iter)
         {
             const CPLString &osExtension = *iter;
-            if (EQUALN(pszFilename + i, osExtension.c_str(),
+            if (EQUALN(osFilenameCopy.c_str() + i, osExtension.c_str(),
                        osExtension.size()))
             {
                 nToSkip = static_cast<int>(osExtension.size());
@@ -470,7 +475,8 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
 
 #ifdef DEBUG
         // For AFL, so that .cur_input is detected as the archive filename.
-        if (EQUALN(pszFilename + i, ".cur_input", strlen(".cur_input")))
+        if (EQUALN(osFilenameCopy.c_str() + i, ".cur_input",
+                   strlen(".cur_input")))
         {
             nToSkip = static_cast<int>(strlen(".cur_input"));
         }
@@ -486,7 +492,7 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
                 break;
             }
             VSIStatBufL statBuf;
-            char *archiveFilename = CPLStrdup(pszFilename);
+            char *archiveFilename = CPLStrdup(osFilenameCopy.c_str());
             bool bArchiveFileExists = false;
 
             if (IsEitherSlash(archiveFilename[i + nToSkip]))
@@ -527,10 +533,11 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
 
             if (bArchiveFileExists)
             {
-                if (IsEitherSlash(pszFilename[i + nToSkip]))
+                if (IsEitherSlash(osFilenameCopy[i + nToSkip]) &&
+                    i + nToSkip + 1 < static_cast<int>(osFilenameCopy.size()))
                 {
-                    osFileInArchive =
-                        CompactFilename(pszFilename + i + nToSkip + 1);
+                    osFileInArchive = CompactFilename(osFilenameCopy.c_str() +
+                                                      i + nToSkip + 1);
                 }
                 else
                 {
@@ -545,12 +552,22 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
                         osFileInArchive.pop_back();
                 }
 
+                // Messy! Restore the TLS buffer if it has been altered
+                if (osFilenameCopy != pszFilename)
+                    strcpy(const_cast<char *>(pszFilename),
+                           osFilenameCopy.c_str());
+
                 return archiveFilename;
             }
             CPLFree(archiveFilename);
         }
         i++;
     }
+
+    // Messy! Restore the TLS buffer if it has been altered
+    if (osFilenameCopy != pszFilename)
+        strcpy(const_cast<char *>(pszFilename), osFilenameCopy.c_str());
+
     return nullptr;
 }