Skip to content
Open

fix bug #3118

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
38 changes: 10 additions & 28 deletions rts/Lua/LuaIO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,6 @@
#include "System/Misc/TracyDefs.h"


/******************************************************************************/
/******************************************************************************/

static bool IsSafePath(const std::string& path)
{
RECOIL_DETAILED_TRACY_ZONE;
// keep searches within the Spring directory
if ((path[0] == '/') || (path[0] == '\\') ||
((path.size() >= 2) && (path[1] == ':'))) {
return false;
}
if ((path.find("..") != std::string::npos) ||
(path.find("springsettings.cfg") != std::string::npos) || //don't allow to change config file
(path.find(".springrc") != std::string::npos) ||
(path.find("springrc") != std::string::npos)
) {
return false;
}

return true;
}


/******************************************************************************/
/******************************************************************************/

Expand Down Expand Up @@ -84,6 +61,13 @@ bool LuaIO::SafeWritePath(const std::string& path)
if (std::find(std::begin(exeFiles), std::end(exeFiles), ext) != exeFiles.end())
return false;

// don't allow touching the config files; springsettings can redirect
// the write-dir, which would defeat every other path check here
const std::string lowerPath = StringToLower(path);
if ((lowerPath.find("springsettings.cfg") != std::string::npos) ||
(lowerPath.find("springrc") != std::string::npos))
return false;

return dataDirsAccess.InWriteDir(path);
}

Expand All @@ -100,7 +84,7 @@ FILE* LuaIO::fopen(lua_State* L, const char* path, const char* mode)
errno = EINVAL;
return nullptr;
}
if (!IsSafePath(path)) {
if (!SafeWritePath(path)) {
Comment on lines -103 to +87

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably depend on mode str. if r then check safe read path, if wat check safe write path

errno = EPERM; //EACCESS?
return nullptr;
}
Expand Down Expand Up @@ -141,8 +125,7 @@ int LuaIO::system(lua_State* L, const char* command)
int LuaIO::remove(lua_State* L, const char* pathname)
{
RECOIL_DETAILED_TRACY_ZONE;
if (!SafeWritePath(pathname)
|| !IsSafePath(pathname)) {
if (!SafeWritePath(pathname)) {
errno = EPERM; //EACCESS?
return -1;
}
Expand All @@ -153,8 +136,7 @@ int LuaIO::remove(lua_State* L, const char* pathname)
int LuaIO::rename(lua_State* L, const char* oldpath, const char* newpath)
{
RECOIL_DETAILED_TRACY_ZONE;
if (!SafeWritePath(oldpath) || !SafeWritePath(newpath)
|| !IsSafePath(oldpath) || !IsSafePath(newpath)) {
if (!SafeWritePath(oldpath) || !SafeWritePath(newpath)) {
errno = EPERM; //EACCESS?
return -1;
}
Expand Down
4 changes: 4 additions & 0 deletions rts/Lua/LuaUnsyncedCtrl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "LuaInclude.h"
#include "LuaHandle.h"
#include "LuaHashString.h"
#include "LuaIO.h"
#include "LuaMenu.h"
#include "LuaOpenGLUtils.h"
#include "LuaParser.h"
Expand Down Expand Up @@ -2720,6 +2721,9 @@ int LuaUnsyncedCtrl::ExtractModArchiveFile(lua_State* L)
{
const string path = luaL_checkstring(L, 1);

if (!LuaIO::SafeWritePath(path))
luaL_error(L, "[%s] invalid access: %s", __func__, path.c_str());

CFileHandler vfsFile(path, SPRING_VFS_ZIP);
CFileHandler rawFile(path, SPRING_VFS_RAW);

Expand Down
14 changes: 10 additions & 4 deletions rts/Lua/LuaVFS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,9 @@ int LuaVFS::Include(lua_State* L, bool synced)
ScopedOnceTimer timer("LuaVFS::Include(" + fileName + ")");
#endif

// the path may point to a file or dir outside of any data-dir
// if (!LuaIO::IsSimplePath(fileName)) return 0;
// keep searches within the data directories
if (!LuaIO::SafeWritePath(fileName))
luaL_error(L, "[LuaVFS::%s] invalid access: %s", __func__, fileName.c_str());
Comment on lines +322 to +323

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? this is a read path and not a write path


// note: this check must happen before luaL_loadbuffer gets called
// it pushes new values on the stack and if only index 1 was given
Expand Down Expand Up @@ -416,8 +417,9 @@ int LuaVFS::UnsyncInclude(lua_State* L)
int LuaVFS::LoadFile(lua_State* L, bool synced)
{
const string filename = luaL_checkstring(L, 1);
// the path may point to a file or dir outside of any data-dir
// if (!LuaIO::IsSimplePath(filename)) return 0;
// keep searches within the data directories
if (!LuaIO::SafeWritePath(filename))
return 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this one not do luaL_error like all the others? also this is a read path too


string data;
if (LoadFileWithModes(filename, data, GetModes(L, 2, synced)) == 1) {
Expand Down Expand Up @@ -836,6 +838,10 @@ int LuaVFS::CompressFolder(lua_State* L)
const std::string& compressedFilePath = luaL_optstring(L, 3, (folderPath + ".sdz").c_str());
const std::string& modes = GetModes(L, 5, false);

// keep the output within the writable data-directory
if (!LuaIO::SafeWritePath(compressedFilePath))
luaL_error(L, "[LuaVFS::%s] invalid access: %s", __func__, compressedFilePath.c_str());

const bool includeFolder = luaL_optboolean(L, 4, false);

if (CFileHandler::FileExists(compressedFilePath, modes))
Expand Down