From: ShadowNinja <shadowninja@minetest.net>
Date: Fri, 16 Dec 2016 22:43:39 +0000 (-0500)
Subject: Security: Fix resolving of some relative paths
X-Git-Tag: 0.4.15~16
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=0f0502109eac44128e87906fff30b5d049392f1d;p=oweals%2Fminetest.git

Security: Fix resolving of some relative paths

Trying to resolve a path with RemoveRelativePathComponents that can't
be resolved without leaving leading parent components (e.g. "../worlds/foo"
or "bar/../../worlds/foo") will fail.  To work around this, we leave
the relative components and simply remove the trailing components one
at a time, and bail out when we find a parent component.  This will
still fail for paths like "worlds/foo/noexist/../auth.txt" (the path
before the last parent component must not exist), but this is fine
since you won't be able to open a file with a path like that anyways
(the O.S. will determine that the path doesn't exist.
Try `cat /a/../etc/passwd`).
---

diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp
index b915747c6..1b1f148cd 100644
--- a/src/script/cpp_api/s_security.cpp
+++ b/src/script/cpp_api/s_security.cpp
@@ -344,8 +344,7 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path,
 
 	std::string str;  // Transient
 
-	std::string norel_path = fs::RemoveRelativePathComponents(path);
-	std::string abs_path = fs::AbsolutePath(norel_path);
+	std::string abs_path = fs::AbsolutePath(path);
 
 	if (!abs_path.empty()) {
 		// Don't allow accessing the settings file
@@ -356,18 +355,29 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path,
 	// If we couldn't find the absolute path (path doesn't exist) then
 	// try removing the last components until it works (to allow
 	// non-existent files/folders for mkdir).
-	std::string cur_path = norel_path;
+	std::string cur_path = path;
 	std::string removed;
 	while (abs_path.empty() && !cur_path.empty()) {
-		std::string tmp_rmed;
-		cur_path = fs::RemoveLastPathComponent(cur_path, &tmp_rmed);
-		removed = tmp_rmed + (removed.empty() ? "" : DIR_DELIM + removed);
+		std::string component;
+		cur_path = fs::RemoveLastPathComponent(cur_path, &component);
+		if (component == "..") {
+			// Parent components can't be allowed or we could allow something like
+			// /home/user/minetest/worlds/foo/noexist/../../../../../../etc/passwd.
+			// If we have previous non-relative elements in the path we might be
+			// able to remove them so that things like worlds/foo/noexist/../auth.txt
+			// could be allowed, but those paths will be interpreted as nonexistent
+			// by the operating system anyways.
+			return false;
+		}
+		removed = component + (removed.empty() ? "" : DIR_DELIM + removed);
 		abs_path = fs::AbsolutePath(cur_path);
 	}
-	if (abs_path.empty()) return false;
+	if (abs_path.empty())
+		return false;
 	// Add the removed parts back so that you can't, eg, create a
 	// directory in worldmods if worldmods doesn't exist.
-	if (!removed.empty()) abs_path += DIR_DELIM + removed;
+	if (!removed.empty())
+		abs_path += DIR_DELIM + removed;
 
 	// Get server from registry
 	lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_SCRIPTAPI);