Skip to content

Commit 11ff175

Browse files
authoredJan 12, 2025
Merge pull request OSGeo#11638 from rouault/fix_ossfuzz_388868487
Fix read heap-buffer-overflow on nested /vsitar/ calls
2 parents edac8cd + c80a461 commit 11ff175

File tree

2 files changed

+36
-11
lines changed

2 files changed

+36
-11
lines changed
 

Diff for: ‎port/cpl_path.cpp

+12-4
Original file line numberDiff line numberDiff line change
@@ -801,8 +801,12 @@ const char *CPLFormCIFilename(const char *pszPath, const char *pszBasename,
801801
pszFilename[i] = static_cast<char>(CPLToupper(pszFilename[i]));
802802
}
803803

804-
pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
805-
nStatRet = VSIStatExL(pszFullPath, &sStatBuf, VSI_STAT_EXISTS_FLAG);
804+
const std::string osTmpPath(
805+
CPLFormFilename(pszPath, pszFilename, nullptr));
806+
nStatRet =
807+
VSIStatExL(osTmpPath.c_str(), &sStatBuf, VSI_STAT_EXISTS_FLAG);
808+
if (nStatRet == 0)
809+
pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
806810
}
807811

808812
if (nStatRet != 0)
@@ -813,8 +817,12 @@ const char *CPLFormCIFilename(const char *pszPath, const char *pszBasename,
813817
CPLTolower(static_cast<unsigned char>(pszFilename[i])));
814818
}
815819

816-
pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
817-
nStatRet = VSIStatExL(pszFullPath, &sStatBuf, VSI_STAT_EXISTS_FLAG);
820+
const std::string osTmpPath(
821+
CPLFormFilename(pszPath, pszFilename, nullptr));
822+
nStatRet =
823+
VSIStatExL(osTmpPath.c_str(), &sStatBuf, VSI_STAT_EXISTS_FLAG);
824+
if (nStatRet == 0)
825+
pszFullPath = CPLFormFilename(pszPath, pszFilename, nullptr);
818826
}
819827

820828
if (nStatRet != 0)

Diff for: ‎port/cpl_vsil_abstract_archive.cpp

+24-7
Original file line numberDiff line numberDiff line change
@@ -452,15 +452,20 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
452452

453453
const std::vector<CPLString> oExtensions = GetExtensions();
454454
int nAttempts = 0;
455-
while (pszFilename[i])
455+
// If we are called with pszFilename being one of the TLS buffers returned
456+
// by cpl_path.cpp functions, then a call to Stat() in the loop (and its
457+
// cascaded calls to other cpl_path.cpp functions) might lead to altering
458+
// the buffer to be altered, hence take a copy
459+
const std::string osFilenameCopy(pszFilename);
460+
while (i < static_cast<int>(osFilenameCopy.size()))
456461
{
457462
int nToSkip = 0;
458463

459464
for (std::vector<CPLString>::const_iterator iter = oExtensions.begin();
460465
iter != oExtensions.end(); ++iter)
461466
{
462467
const CPLString &osExtension = *iter;
463-
if (EQUALN(pszFilename + i, osExtension.c_str(),
468+
if (EQUALN(osFilenameCopy.c_str() + i, osExtension.c_str(),
464469
osExtension.size()))
465470
{
466471
nToSkip = static_cast<int>(osExtension.size());
@@ -470,7 +475,8 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
470475

471476
#ifdef DEBUG
472477
// For AFL, so that .cur_input is detected as the archive filename.
473-
if (EQUALN(pszFilename + i, ".cur_input", strlen(".cur_input")))
478+
if (EQUALN(osFilenameCopy.c_str() + i, ".cur_input",
479+
strlen(".cur_input")))
474480
{
475481
nToSkip = static_cast<int>(strlen(".cur_input"));
476482
}
@@ -486,7 +492,7 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
486492
break;
487493
}
488494
VSIStatBufL statBuf;
489-
char *archiveFilename = CPLStrdup(pszFilename);
495+
char *archiveFilename = CPLStrdup(osFilenameCopy.c_str());
490496
bool bArchiveFileExists = false;
491497

492498
if (IsEitherSlash(archiveFilename[i + nToSkip]))
@@ -527,10 +533,11 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
527533

528534
if (bArchiveFileExists)
529535
{
530-
if (IsEitherSlash(pszFilename[i + nToSkip]))
536+
if (IsEitherSlash(osFilenameCopy[i + nToSkip]) &&
537+
i + nToSkip + 1 < static_cast<int>(osFilenameCopy.size()))
531538
{
532-
osFileInArchive =
533-
CompactFilename(pszFilename + i + nToSkip + 1);
539+
osFileInArchive = CompactFilename(osFilenameCopy.c_str() +
540+
i + nToSkip + 1);
534541
}
535542
else
536543
{
@@ -545,12 +552,22 @@ char *VSIArchiveFilesystemHandler::SplitFilename(const char *pszFilename,
545552
osFileInArchive.pop_back();
546553
}
547554

555+
// Messy! Restore the TLS buffer if it has been altered
556+
if (osFilenameCopy != pszFilename)
557+
strcpy(const_cast<char *>(pszFilename),
558+
osFilenameCopy.c_str());
559+
548560
return archiveFilename;
549561
}
550562
CPLFree(archiveFilename);
551563
}
552564
i++;
553565
}
566+
567+
// Messy! Restore the TLS buffer if it has been altered
568+
if (osFilenameCopy != pszFilename)
569+
strcpy(const_cast<char *>(pszFilename), osFilenameCopy.c_str());
570+
554571
return nullptr;
555572
}
556573

0 commit comments

Comments
 (0)