From 59f84ca0a07e50dd5ce050d38ae1aeb529bd25ac Mon Sep 17 00:00:00 2001 From: ShadowNinja Date: Mon, 5 Dec 2016 19:59:15 +0000 Subject: [PATCH] Mod security: Allow read-only access to all mod paths --- src/script/cpp_api/s_security.cpp | 82 +++++++++++++++++++++--------- src/script/cpp_api/s_security.h | 21 +++++--- src/script/lua_api/l_areastore.cpp | 4 +- src/script/lua_api/l_mapgen.cpp | 2 +- src/script/lua_api/l_settings.cpp | 13 +++-- src/script/lua_api/l_settings.h | 3 +- src/script/lua_api/l_util.cpp | 4 +- src/server.h | 1 + 8 files changed, 89 insertions(+), 41 deletions(-) diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp index c9816f89b..b915747c6 100644 --- a/src/script/cpp_api/s_security.cpp +++ b/src/script/cpp_api/s_security.cpp @@ -336,8 +336,12 @@ bool ScriptApiSecurity::safeLoadFile(lua_State *L, const char *path) } -bool ScriptApiSecurity::checkPath(lua_State *L, const char *path) +bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, + bool write_required, bool *write_allowed) { + if (write_allowed) + *write_allowed = false; + std::string str; // Transient std::string norel_path = fs::RemoveRelativePathComponents(path); @@ -380,32 +384,53 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path) // Builtin can access anything if (mod_name == BUILTIN_MOD_NAME) { + if (write_allowed) *write_allowed = true; return true; } // Allow paths in mod path - const ModSpec *mod = server->getModSpec(mod_name); - if (mod) { - str = fs::AbsolutePath(mod->path); + // Don't bother if write access isn't important, since it will be handled later + if (write_required || write_allowed != NULL) { + const ModSpec *mod = server->getModSpec(mod_name); + if (mod) { + str = fs::AbsolutePath(mod->path); + if (!str.empty() && fs::PathStartsWith(abs_path, str)) { + if (write_allowed) *write_allowed = true; + return true; + } + } + } + } + lua_pop(L, 1); // Pop mod name + + // Allow read-only access to all mod directories + if (!write_required) { + const std::vector mods = server->getMods(); + for (size_t i = 0; i < mods.size(); ++i) { + str = fs::AbsolutePath(mods[i].path); if (!str.empty() && fs::PathStartsWith(abs_path, str)) { return true; } } } - lua_pop(L, 1); // Pop mod name str = fs::AbsolutePath(server->getWorldPath()); - if (str.empty()) return false; - // Don't allow access to world mods. We add to the absolute path - // of the world instead of getting the absolute paths directly - // because that won't work if they don't exist. - if (fs::PathStartsWith(abs_path, str + DIR_DELIM + "worldmods") || - fs::PathStartsWith(abs_path, str + DIR_DELIM + "game")) { - return false; - } - // Allow all other paths in world path - if (fs::PathStartsWith(abs_path, str)) { - return true; + if (!str.empty()) { + // Don't allow access to other paths in the world mod/game path. + // These have to be blocked so you can't override a trusted mod + // by creating a mod with the same name in a world mod directory. + // We add to the absolute path of the world instead of getting + // the absolute paths directly because that won't work if they + // don't exist. + if (fs::PathStartsWith(abs_path, str + DIR_DELIM + "worldmods") || + fs::PathStartsWith(abs_path, str + DIR_DELIM + "game")) { + return false; + } + // Allow all other paths in world path + if (fs::PathStartsWith(abs_path, str)) { + if (write_allowed) *write_allowed = true; + return true; + } } // Default to disallowing @@ -476,7 +501,7 @@ int ScriptApiSecurity::sl_g_loadfile(lua_State *L) if (lua_isstring(L, 1)) { path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + CHECK_SECURE_PATH_INTERNAL(L, path, false, NULL); } if (!safeLoadFile(L, path)) { @@ -529,7 +554,16 @@ int ScriptApiSecurity::sl_io_open(lua_State *L) luaL_checktype(L, 1, LUA_TSTRING); const char *path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + + bool write_requested = false; + if (with_mode) { + luaL_checktype(L, 2, LUA_TSTRING); + const char *mode = lua_tostring(L, 2); + write_requested = strchr(mode, 'w') != NULL || + strchr(mode, '+') != NULL || + strchr(mode, 'a') != NULL; + } + CHECK_SECURE_PATH_INTERNAL(L, path, write_requested, NULL); push_original(L, "io", "open"); lua_pushvalue(L, 1); @@ -546,7 +580,7 @@ int ScriptApiSecurity::sl_io_input(lua_State *L) { if (lua_isstring(L, 1)) { const char *path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + CHECK_SECURE_PATH_INTERNAL(L, path, false, NULL); } push_original(L, "io", "input"); @@ -560,7 +594,7 @@ int ScriptApiSecurity::sl_io_output(lua_State *L) { if (lua_isstring(L, 1)) { const char *path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + CHECK_SECURE_PATH_INTERNAL(L, path, true, NULL); } push_original(L, "io", "output"); @@ -574,7 +608,7 @@ int ScriptApiSecurity::sl_io_lines(lua_State *L) { if (lua_isstring(L, 1)) { const char *path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + CHECK_SECURE_PATH_INTERNAL(L, path, false, NULL); } int top_precall = lua_gettop(L); @@ -591,11 +625,11 @@ int ScriptApiSecurity::sl_os_rename(lua_State *L) { luaL_checktype(L, 1, LUA_TSTRING); const char *path1 = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path1); + CHECK_SECURE_PATH_INTERNAL(L, path1, true, NULL); luaL_checktype(L, 2, LUA_TSTRING); const char *path2 = lua_tostring(L, 2); - CHECK_SECURE_PATH(L, path2); + CHECK_SECURE_PATH_INTERNAL(L, path2, true, NULL); push_original(L, "os", "rename"); lua_pushvalue(L, 1); @@ -609,7 +643,7 @@ int ScriptApiSecurity::sl_os_remove(lua_State *L) { luaL_checktype(L, 1, LUA_TSTRING); const char *path = lua_tostring(L, 1); - CHECK_SECURE_PATH(L, path); + CHECK_SECURE_PATH_INTERNAL(L, path, true, NULL); push_original(L, "os", "remove"); lua_pushvalue(L, 1); diff --git a/src/script/cpp_api/s_security.h b/src/script/cpp_api/s_security.h index 97bc5c067..6876108e8 100644 --- a/src/script/cpp_api/s_security.h +++ b/src/script/cpp_api/s_security.h @@ -23,14 +23,18 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "cpp_api/s_base.h" -#define CHECK_SECURE_PATH(L, path) \ - if (!ScriptApiSecurity::checkPath(L, path)) { \ - throw LuaError(std::string("Attempt to access external file ") + \ - path + " with mod security on."); \ +#define CHECK_SECURE_PATH_INTERNAL(L, path, write_required, ptr) \ + if (!ScriptApiSecurity::checkPath(L, path, write_required, ptr)) { \ + throw LuaError(std::string("Mod security: Blocked attempted ") + \ + (write_required ? "write to " : "read from ") + path); \ } -#define CHECK_SECURE_PATH_OPTIONAL(L, path) \ +#define CHECK_SECURE_PATH(L, path, write_required) \ if (ScriptApiSecurity::isSecure(L)) { \ - CHECK_SECURE_PATH(L, path); \ + CHECK_SECURE_PATH_INTERNAL(L, path, write_required, NULL); \ + } +#define CHECK_SECURE_PATH_POSSIBLE_WRITE(L, path, ptr) \ + if (ScriptApiSecurity::isSecure(L)) { \ + CHECK_SECURE_PATH_INTERNAL(L, path, false, ptr); \ } @@ -43,8 +47,9 @@ public: static bool isSecure(lua_State *L); // Loads a file as Lua code safely (doesn't allow bytecode). static bool safeLoadFile(lua_State *L, const char *path); - // Checks if mods are allowed to read and write to the path - static bool checkPath(lua_State *L, const char *path); + // Checks if mods are allowed to read (and optionally write) to the path + static bool checkPath(lua_State *L, const char *path, bool write_required, + bool *write_allowed=NULL); private: // Syntax: "sl_" '_' diff --git a/src/script/lua_api/l_areastore.cpp b/src/script/lua_api/l_areastore.cpp index 0912e2ab0..09a5c78f9 100644 --- a/src/script/lua_api/l_areastore.cpp +++ b/src/script/lua_api/l_areastore.cpp @@ -263,7 +263,7 @@ int LuaAreaStore::l_to_file(lua_State *L) AreaStore *ast = o->as; const char *filename = luaL_checkstring(L, 2); - CHECK_SECURE_PATH_OPTIONAL(L, filename); + CHECK_SECURE_PATH(L, filename, true); std::ostringstream os(std::ios_base::binary); ast->serialize(os); @@ -294,7 +294,7 @@ int LuaAreaStore::l_from_file(lua_State *L) LuaAreaStore *o = checkobject(L, 1); const char *filename = luaL_checkstring(L, 2); - CHECK_SECURE_PATH_OPTIONAL(L, filename); + CHECK_SECURE_PATH(L, filename, false); std::ifstream is(filename, std::ios::binary); return deserialization_helper(L, o->as, is); diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 281f68e46..bc1c32f03 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -1295,7 +1295,7 @@ int ModApiMapgen::l_create_schematic(lua_State *L) INodeDefManager *ndef = getServer(L)->getNodeDefManager(); const char *filename = luaL_checkstring(L, 4); - CHECK_SECURE_PATH_OPTIONAL(L, filename); + CHECK_SECURE_PATH(L, filename, true); Map *map = &(getEnv(L)->getMap()); Schematic schem; diff --git a/src/script/lua_api/l_settings.cpp b/src/script/lua_api/l_settings.cpp index 35b82b435..ea3d50857 100644 --- a/src/script/lua_api/l_settings.cpp +++ b/src/script/lua_api/l_settings.cpp @@ -118,6 +118,11 @@ int LuaSettings::l_write(lua_State* L) NO_MAP_LOCK_REQUIRED; LuaSettings* o = checkobject(L, 1); + if (!o->m_write_allowed) { + throw LuaError("Settings: writing " + o->m_filename + + " not allowed with mod security on."); + } + bool success = o->m_settings->updateConfigFile(o->m_filename.c_str()); lua_pushboolean(L, success); @@ -142,8 +147,9 @@ int LuaSettings::l_to_table(lua_State* L) return 1; } -LuaSettings::LuaSettings(const char* filename) +LuaSettings::LuaSettings(const char* filename, bool write_allowed) { + m_write_allowed = write_allowed; m_filename = std::string(filename); m_settings = new Settings(); @@ -188,9 +194,10 @@ void LuaSettings::Register(lua_State* L) int LuaSettings::create_object(lua_State* L) { NO_MAP_LOCK_REQUIRED; + bool write_allowed; const char* filename = luaL_checkstring(L, 1); - CHECK_SECURE_PATH_OPTIONAL(L, filename); - LuaSettings* o = new LuaSettings(filename); + CHECK_SECURE_PATH_POSSIBLE_WRITE(L, filename, &write_allowed); + LuaSettings* o = new LuaSettings(filename, write_allowed); *(void **)(lua_newuserdata(L, sizeof(void *))) = o; luaL_getmetatable(L, className); lua_setmetatable(L, -2); diff --git a/src/script/lua_api/l_settings.h b/src/script/lua_api/l_settings.h index cb0c09a73..bca333e31 100644 --- a/src/script/lua_api/l_settings.h +++ b/src/script/lua_api/l_settings.h @@ -53,11 +53,12 @@ private: // to_table(self) -> {[key1]=value1,...} static int l_to_table(lua_State* L); + bool m_write_allowed; Settings* m_settings; std::string m_filename; public: - LuaSettings(const char* filename); + LuaSettings(const char* filename, bool write_allowed); ~LuaSettings(); // LuaSettings(filename) diff --git a/src/script/lua_api/l_util.cpp b/src/script/lua_api/l_util.cpp index 818c1aeeb..26e2b985c 100644 --- a/src/script/lua_api/l_util.cpp +++ b/src/script/lua_api/l_util.cpp @@ -388,7 +388,7 @@ int ModApiUtil::l_mkdir(lua_State *L) { NO_MAP_LOCK_REQUIRED; const char *path = luaL_checkstring(L, 1); - CHECK_SECURE_PATH_OPTIONAL(L, path); + CHECK_SECURE_PATH(L, path, true); lua_pushboolean(L, fs::CreateAllDirs(path)); return 1; } @@ -400,7 +400,7 @@ int ModApiUtil::l_get_dir_list(lua_State *L) const char *path = luaL_checkstring(L, 1); short is_dir = lua_isboolean(L, 2) ? lua_toboolean(L, 2) : -1; - CHECK_SECURE_PATH_OPTIONAL(L, path); + CHECK_SECURE_PATH(L, path, false); std::vector list = fs::GetDirListing(path); diff --git a/src/server.h b/src/server.h index cc2bcef25..4425d139b 100644 --- a/src/server.h +++ b/src/server.h @@ -298,6 +298,7 @@ public: IWritableNodeDefManager* getWritableNodeDefManager(); IWritableCraftDefManager* getWritableCraftDefManager(); + const std::vector &getMods() const { return m_mods; } const ModSpec* getModSpec(const std::string &modname) const; void getModNames(std::vector &modlist); std::string getBuiltinLuaPath(); -- 2.25.1