diff --git a/OpenClaw/Engine/Resource/ResourceCache.cpp b/OpenClaw/Engine/Resource/ResourceCache.cpp index 2abff8d2a3..f6315778bc 100644 --- a/OpenClaw/Engine/Resource/ResourceCache.cpp +++ b/OpenClaw/Engine/Resource/ResourceCache.cpp @@ -1,11 +1,27 @@ #include #include +#include #include #include "ResourceCache.h" #include "../Util/StringUtil.h" +// -------------------------------------------------------------------------- +// Function: IsValidResourcePath +// Purpose: Reject resource paths that contain ".." components or +// embedded null bytes, blocking directory traversal attacks +// via crafted archive filenames. +// -------------------------------------------------------------------------- +bool IsValidResourcePath(const std::string& path) +{ + if (path.find("..") != std::string::npos) + return false; + if (path.find('\0') != std::string::npos) + return false; + return true; +} + // // Resource::Resource // @@ -51,6 +67,12 @@ bool ResourceRezArchive::VOpen() int32 ResourceRezArchive::VGetRawResourceSize(Resource* r) { + if (!IsValidResourcePath(r->GetName())) + { + LOG_ERROR("Blocked unsafe resource path: " + r->GetName()); + return -1; + } + RezFile* rezFile = WAP_GetRezFileFromRezArchive(_rezArchive, r->GetName().c_str()); if (rezFile == NULL) { @@ -64,6 +86,12 @@ int32 ResourceRezArchive::VGetRawResourceSize(Resource* r) // Remark: outBuffer is valid heap pointer int32 ResourceRezArchive::VGetRawResource(Resource* r, char* outBuffer) { + if (!IsValidResourcePath(r->GetName())) + { + LOG_ERROR("Blocked unsafe resource path: " + r->GetName()); + return -1; + } + assert(outBuffer != NULL); RezFile* rezFile = WAP_GetRezFileFromRezArchive(_rezArchive, r->GetName().c_str()); diff --git a/OpenClaw/Engine/Resource/ResourceCache.h b/OpenClaw/Engine/Resource/ResourceCache.h index 9a62b36ed0..7f0effe854 100644 --- a/OpenClaw/Engine/Resource/ResourceCache.h +++ b/OpenClaw/Engine/Resource/ResourceCache.h @@ -159,6 +159,10 @@ typedef std::list> ResourceHandleList; typedef std::list> ResourceLoaderList; typedef std::map> ResourceHandleMap; +// Returns false if the path contains ".." components or embedded null bytes, +// which could be used to escape the resource archive root (directory traversal). +bool IsValidResourcePath(const std::string& path); + class ResourceCache { public: diff --git a/OpenClaw/Engine/Resource/ZipFile.cpp b/OpenClaw/Engine/Resource/ZipFile.cpp index f90a891790..0b9720b1d6 100644 --- a/OpenClaw/Engine/Resource/ZipFile.cpp +++ b/OpenClaw/Engine/Resource/ZipFile.cpp @@ -9,7 +9,7 @@ // // This program is distributed in the hope that it will be useful, // but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See // http://www.gnu.org/licenses/lgpl-3.0.txt for more details. // // You should have received a copy of the GNU Lesser GPL v3 @@ -190,6 +190,16 @@ bool ZipFile::Init(const std::string &resFileName) { pfh += sizeof(fh); + // Security fix: reject ZIP entries with filenames that would overflow + // a _MAX_PATH-sized buffer. fnameLen is a uint16 (max 65535) read + // directly from the archive, so a malformed ZIP could previously + // cause a stack-buffer overflow here. + if (fh.fnameLen >= _MAX_PATH) + { + success = false; + break; + } + char fileName[_MAX_PATH]; memcpy(fileName, pfh, fh.fnameLen); fileName[fh.fnameLen] = 0; @@ -273,7 +283,7 @@ std::vector ZipFile::GetAllFilesInDirectory(const std::string& dir // -------------------------------------------------------------------------- // Function: End // Purpose: Finish the object -// Parameters: +// Parameters: // -------------------------------------------------------------------------- void ZipFile::End() { @@ -292,10 +302,12 @@ std::string ZipFile::GetFilename(int i) const std::string fileName = ""; if (i >= 0 && i < m_nEntries) { - char pszDest[_MAX_PATH]; - memcpy(pszDest, m_papDir[i]->GetName(), m_papDir[i]->fnameLen); - pszDest[m_papDir[i]->fnameLen] = '\0'; - fileName = pszDest; + // Security fix: use std::string constructor with explicit length instead + // of copying into a fixed _MAX_PATH stack buffer. The old code wrote + // pszDest[fnameLen] = '\0' which was an out-of-bounds write whenever + // fnameLen >= _MAX_PATH (260), since fnameLen is a uint16 read from the + // untrusted ZIP archive and can be up to 65535. + fileName = std::string(m_papDir[i]->GetName(), m_papDir[i]->fnameLen); if (fileName.size() > 0 && fileName[0] != '/') {