diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp index cadeafdd1..7c8ba8931 100644 --- a/src/script/cpp_api/s_security.cpp +++ b/src/script/cpp_api/s_security.cpp @@ -505,6 +505,28 @@ bool ScriptApiSecurity::safeLoadFile(lua_State *L, const char *path, const char } +bool checkModNameWhitelisted(const std::string &mod_name, const std::string &setting) +{ + assert(str_starts_with(setting, "secure.")); + + if (mod_name.empty()) + return false; + + std::string value = g_settings->get(setting); + value.erase(std::remove(value.begin(), value.end(), ' '), value.end()); + auto mod_list = str_split(value, ','); + + return CONTAINS(mod_list, mod_name); +} + + +bool ScriptApiSecurity::checkWhitelisted(lua_State *L, const std::string &setting) +{ + std::string mod_name = ScriptApiBase::getCurrentModName(L); + return checkModNameWhitelisted(mod_name, setting); +} + + bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, bool write_required, bool *write_allowed) { @@ -555,7 +577,6 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, return false; // Get mod name - // FIXME: insecure = problem here? std::string mod_name = ScriptApiBase::getCurrentModNameInsecure(L); if (!mod_name.empty()) { // Builtin can access anything @@ -571,7 +592,32 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, if (mod) { str = fs::AbsolutePath(mod->path); if (!str.empty() && fs::PathStartsWith(abs_path, str)) { - if (write_allowed) *write_allowed = true; + // `mod_name` cannot be trusted here, so we catch the scenarios where this becomes a problem: + bool is_trusted = checkModNameWhitelisted(mod_name, "secure.trusted_mods") || + checkModNameWhitelisted(mod_name, "secure.http_mods"); + std::string filename = lowercase(fs::GetFilenameFromPath(abs_path.c_str())); + // By writing to any of these a malicious mod could turn itself into + // an existing trusted mod by renaming or becoming a modpack. + bool is_dangerous_file = filename == "mod.conf" || + filename == "modpack.conf" || + filename == "modpack.txt"; + if (write_required) { + if (is_trusted) { + throw LuaError( + "Unable to write to a trusted or http mod's directory. " + "For data storage consider minetest.get_mod_data_path() or minetest.get_worldpath() instead."); + } else if (is_dangerous_file) { + throw LuaError( + "Unable to write to special file for security reasons"); + } else { + const char *message = + "Writing to mod directories is deprecated, as any changes " + "will be overwritten when updating content. " + "For data storage consider minetest.get_mod_data_path() or minetest.get_worldpath() instead."; + log_deprecated(L, message, 1); + } + } + if (write_allowed) *write_allowed = !is_trusted && !is_dangerous_file; return true; } } @@ -630,21 +676,6 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, return false; } -bool ScriptApiSecurity::checkWhitelisted(lua_State *L, const std::string &setting) -{ - assert(str_starts_with(setting, "secure.")); - - std::string mod_name = ScriptApiBase::getCurrentModName(L); - if (mod_name.empty()) - return false; - - std::string value = g_settings->get(setting); - value.erase(std::remove(value.begin(), value.end(), ' '), value.end()); - auto mod_list = str_split(value, ','); - - return CONTAINS(mod_list, mod_name); -} - int ScriptApiSecurity::sl_g_dofile(lua_State *L) { diff --git a/src/script/cpp_api/s_security.h b/src/script/cpp_api/s_security.h index 880ce1638..ee93e1f99 100644 --- a/src/script/cpp_api/s_security.h +++ b/src/script/cpp_api/s_security.h @@ -49,12 +49,12 @@ public: static bool safeLoadString(lua_State *L, const std::string &code, const char *chunk_name); // Loads a file as Lua code safely (doesn't allow bytecode). static bool safeLoadFile(lua_State *L, const char *path, const char *display_name = NULL); - // 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); // Check if mod is whitelisted in the given setting // This additionally checks that the mod's main file scope is executing. static bool checkWhitelisted(lua_State *L, const std::string &setting); + // 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: int getThread(lua_State *L);