Mod security: Allow read-only access to all mod paths

diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp
index c9816f89bd512fafbf105476577b61901d8c6f1e..b915747c6929cf77db0d309c1769e2ac06ef80f7 100644 (file)
--- 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<ModSpec> 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 97bc5c0671bc5876d25873f171f191e7f4c67805..6876108e8f6fcf2ff4fa1ff3e0ae923181f75d16 100644 (file)
--- 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_" <Library name or 'g' (global)> '_' <Function name>

diff --git a/src/script/lua_api/l_areastore.cpp b/src/script/lua_api/l_areastore.cpp
index 0912e2ab0bbc9f878275356ed8fef24797f44ab5..09a5c78f98a544da184f071d3458b7684bd707f9 100644 (file)
--- 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 281f68e46cd5643ce6518815f902ea83daa548d8..bc1c32f030c15a42a464c6171fc2fdf68048d3c2 100644 (file)
--- 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 35b82b435cc677b9f47f8bd8a7316bc9668b4021..ea3d508575a540f8e23ee0ccf835cdbf089259ad 100644 (file)
--- 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 cb0c09a734a9eb3122dd81989a652c90c53aa2ed..bca333e31d2c42d2269724169f5985f350af96da 100644 (file)
--- 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 818c1aeeb2f580097f375b45f3ad5135b38b8855..26e2b985cd87e554730b1c3bec9f23f130614298 100644 (file)
--- 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<fs::DirListNode> list = fs::GetDirListing(path);
 

diff --git a/src/server.h b/src/server.h
index cc2bcef25f916c708369eba63b09e75a52c746ef..4425d139b1658860d643c7243b339c468c341396 100644 (file)
--- a/src/server.h
+++ b/src/server.h
@@ -298,6 +298,7 @@ public:
        IWritableNodeDefManager* getWritableNodeDefManager();
        IWritableCraftDefManager* getWritableCraftDefManager();
 
+       const std::vector<ModSpec> &getMods() const { return m_mods; }
        const ModSpec* getModSpec(const std::string &modname) const;
        void getModNames(std::vector<std::string> &modlist);
        std::string getBuiltinLuaPath();