From 4966cbbe4e9cecfda22bfe1b9950c0ee0c82b4a9 Mon Sep 17 00:00:00 2001 From: Anarchid Date: Sat, 18 Jul 2026 18:24:49 +0300 Subject: [PATCH] Gate Lua io/os/VFS file access behind SafeWritePath Replace the weaker IsSafePath check with SafeWritePath for io.open, os.remove/os.rename, and enable gating for VFS.Include/LoadFile/ CompressFolder and Spring.ExtractModArchiveFile. Fold the springsettings.cfg/springrc blocking into SafeWritePath itself, since the config file can redirect the write-dir and defeat all other checks. Co-Authored-By: Claude Fable 5 --- rts/Lua/LuaIO.cpp | 38 ++++++++++--------------------------- rts/Lua/LuaUnsyncedCtrl.cpp | 4 ++++ rts/Lua/LuaVFS.cpp | 14 ++++++++++---- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/rts/Lua/LuaIO.cpp b/rts/Lua/LuaIO.cpp index 9ed7281375d..cf6423ed2d5 100644 --- a/rts/Lua/LuaIO.cpp +++ b/rts/Lua/LuaIO.cpp @@ -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; -} - - /******************************************************************************/ /******************************************************************************/ @@ -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); } @@ -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)) { errno = EPERM; //EACCESS? return nullptr; } @@ -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; } @@ -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; } diff --git a/rts/Lua/LuaUnsyncedCtrl.cpp b/rts/Lua/LuaUnsyncedCtrl.cpp index 2079d9724ce..fecca3a7f68 100644 --- a/rts/Lua/LuaUnsyncedCtrl.cpp +++ b/rts/Lua/LuaUnsyncedCtrl.cpp @@ -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" @@ -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); diff --git a/rts/Lua/LuaVFS.cpp b/rts/Lua/LuaVFS.cpp index 1c2bab2796b..81d284d8f02 100644 --- a/rts/Lua/LuaVFS.cpp +++ b/rts/Lua/LuaVFS.cpp @@ -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()); // note: this check must happen before luaL_loadbuffer gets called // it pushes new values on the stack and if only index 1 was given @@ -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; string data; if (LoadFileWithModes(filename, data, GetModes(L, 2, synced)) == 1) { @@ -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))