Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions OpenClaw/Engine/Resource/ResourceCache.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
#include <algorithm>
#include <cctype>
#include <string>

#include <libwap.h>

#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
//
Expand Down Expand Up @@ -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)
{
Expand All @@ -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());
Expand Down
4 changes: 4 additions & 0 deletions OpenClaw/Engine/Resource/ResourceCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ typedef std::list<std::shared_ptr<ResourceHandle>> ResourceHandleList;
typedef std::list<std::shared_ptr<IResourceLoader>> ResourceLoaderList;
typedef std::map<std::string, std::shared_ptr<ResourceHandle>> 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:
Expand Down
24 changes: 18 additions & 6 deletions OpenClaw/Engine/Resource/ZipFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -273,7 +283,7 @@ std::vector<std::string> ZipFile::GetAllFilesInDirectory(const std::string& dir
// --------------------------------------------------------------------------
// Function: End
// Purpose: Finish the object
// Parameters:
// Parameters:
// --------------------------------------------------------------------------
void ZipFile::End()
{
Expand All @@ -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] != '/')
{
Expand Down