Deprecate writing to mod directories (#14486)

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)
 {

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);