diff --git a/src/filesys.cpp b/src/filesys.cpp index 8881eb2ca..a368bc697 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -833,16 +833,51 @@ std::string RemoveRelativePathComponents(std::string path) std::string AbsolutePath(const std::string &path) { #ifdef _WIN32 + // handle behavior differences on windows + if (path.empty()) + return ""; + else if (!PathExists(path)) + return ""; char *abs_path = _fullpath(NULL, path.c_str(), MAX_PATH); #else char *abs_path = realpath(path.c_str(), NULL); #endif - if (!abs_path) return ""; + if (!abs_path) + return ""; std::string abs_path_str(abs_path); free(abs_path); return abs_path_str; } +std::string AbsolutePathPartial(const std::string &path) +{ + if (path.empty()) + return ""; + // Try to determine absolute path + std::string abs_path = fs::AbsolutePath(path); + if (!abs_path.empty()) + return abs_path; + // Remove components until it works + std::string cur_path = path; + std::string removed; + while (abs_path.empty() && !cur_path.empty()) { + std::string component; + cur_path = RemoveLastPathComponent(cur_path, &component); + removed = component + (removed.empty() ? "" : DIR_DELIM + removed); + abs_path = AbsolutePath(cur_path); + } + // If we had a relative path that does not exist, it needs to be joined with cwd + if (cur_path.empty() && !IsPathAbsolute(path)) + abs_path = AbsolutePath("."); + // or there's an error + if (abs_path.empty()) + return ""; + // Put them back together and resolve the remaining relative components + if (!removed.empty()) + abs_path.append(DIR_DELIM).append(removed); + return RemoveRelativePathComponents(abs_path); +} + const char *GetFilenameFromPath(const char *path) { const char *filename = strrchr(path, DIR_DELIM_CHAR); diff --git a/src/filesys.h b/src/filesys.h index dededcdb3..0e974d822 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -131,6 +131,12 @@ std::string RemoveRelativePathComponents(std::string path); // components and symlinks removed. Returns "" on error. std::string AbsolutePath(const std::string &path); +// This is a combination of RemoveRelativePathComponents() and AbsolutePath() +// It will resolve symlinks for the leading path components that exist and +// still remove "." and ".." in the rest of the path. +// Returns "" on error. +std::string AbsolutePathPartial(const std::string &path); + // Returns the filename from a path or the entire path if no directory // delimiter is found. const char *GetFilenameFromPath(const char *path); diff --git a/src/porting.cpp b/src/porting.cpp index f0e2bee0b..faef75b7c 100644 --- a/src/porting.cpp +++ b/src/porting.cpp @@ -688,6 +688,10 @@ void initializePaths() #endif // RUN_IN_PLACE + assert(!path_share.empty()); + assert(!path_user.empty()); + assert(!path_cache.empty()); + infostream << "Detected share path: " << path_share << std::endl; infostream << "Detected user path: " << path_user << std::endl; infostream << "Detected cache path: " << path_cache << std::endl; diff --git a/src/porting.h b/src/porting.h index edbc236a8..d6cec6c1a 100644 --- a/src/porting.h +++ b/src/porting.h @@ -109,8 +109,7 @@ extern std::string path_cache; bool getCurrentExecPath(char *buf, size_t len); /* - Get full path of stuff in data directory. - Example: "stone.png" -> "../data/stone.png" + Concatenate subpath to path_share. */ std::string getDataPath(const char *subpath); diff --git a/src/script/cpp_api/s_security.cpp b/src/script/cpp_api/s_security.cpp index 40a95ca1a..38094ab8c 100644 --- a/src/script/cpp_api/s_security.cpp +++ b/src/script/cpp_api/s_security.cpp @@ -566,38 +566,23 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path, if (write_allowed) *write_allowed = false; - std::string abs_path = fs::AbsolutePath(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 = path; - std::string removed; - while (abs_path.empty() && !cur_path.empty()) { - 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; - // Add the removed parts back so that you can e.g. create a - // directory in worldmods if worldmods doesn't exist. - if (!removed.empty()) - abs_path += DIR_DELIM + removed; - + // We can't use AbsolutePath() here since we want to allow creating paths that + // do not yet exist. But RemoveRelativePathComponents() would also be incorrect + // since that wouldn't normalize subpaths that *do* exist. + // This is required so that comparisons with other normalized paths work correctly. + std::string abs_path = fs::AbsolutePathPartial(path); tracestream << "ScriptApiSecurity: path \"" << path << "\" resolved to \"" << abs_path << "\"" << std::endl; + if (abs_path.empty()) + return false; + + // Note: abs_path can be a valid path while path isn't, e.g. + // abs_path = "/home/user/.luanti" + // path = "/home/user/.luanti/noexist/.." + // Letting this through the sandbox isn't a concern as any actual attempts to + // use the path would fail. + // Ask the environment-specific implementation auto *sec = ModApiBase::getScriptApi(L); return sec->checkPathInternal(abs_path, write_required, write_allowed); @@ -617,9 +602,11 @@ bool ScriptApiSecurity::checkPathWithGamedef(lua_State *L, if (!gamedef) return false; - if (!abs_path.empty()) { + assert(!abs_path.empty()); + + if (!g_settings_path.empty()) { // Don't allow accessing the settings file - str = fs::AbsolutePath(g_settings_path); + str = fs::AbsolutePathPartial(g_settings_path); if (str == abs_path) return false; } diff --git a/src/script/scripting_mainmenu.cpp b/src/script/scripting_mainmenu.cpp index d5434c325..88f80a001 100644 --- a/src/script/scripting_mainmenu.cpp +++ b/src/script/scripting_mainmenu.cpp @@ -76,10 +76,11 @@ void MainMenuScripting::registerLuaClasses(lua_State *L, int top) bool MainMenuScripting::mayModifyPath(const std::string &path) { - if (fs::PathStartsWith(path, fs::TempPath())) + std::string path_temp = fs::AbsolutePathPartial(fs::TempPath()); + if (fs::PathStartsWith(path, path_temp)) return true; - std::string path_user = fs::RemoveRelativePathComponents(porting::path_user); + std::string path_user = fs::AbsolutePathPartial(porting::path_user); if (fs::PathStartsWith(path, path_user + DIR_DELIM "client")) return true; @@ -92,7 +93,7 @@ bool MainMenuScripting::mayModifyPath(const std::string &path) if (fs::PathStartsWith(path, path_user + DIR_DELIM "worlds")) return true; - if (fs::PathStartsWith(path, fs::RemoveRelativePathComponents(porting::path_cache))) + if (fs::PathStartsWith(path, fs::AbsolutePathPartial(porting::path_cache))) return true; return false; diff --git a/src/unittest/test_filesys.cpp b/src/unittest/test_filesys.cpp index 9bab4fb5d..34666f611 100644 --- a/src/unittest/test_filesys.cpp +++ b/src/unittest/test_filesys.cpp @@ -24,6 +24,7 @@ public: void testRemoveLastPathComponent(); void testRemoveLastPathComponentWithTrailingDelimiter(); void testRemoveRelativePathComponent(); + void testAbsolutePath(); void testSafeWriteToFile(); void testCopyFileContents(); void testNonExist(); @@ -39,6 +40,7 @@ void TestFileSys::runTests(IGameDef *gamedef) TEST(testRemoveLastPathComponent); TEST(testRemoveLastPathComponentWithTrailingDelimiter); TEST(testRemoveRelativePathComponent); + TEST(testAbsolutePath); TEST(testSafeWriteToFile); TEST(testCopyFileContents); TEST(testNonExist); @@ -55,7 +57,7 @@ static std::string p(std::string path) for (size_t i = 0; i < path.size(); ++i) { if (path[i] == '/') { path.replace(i, 1, DIR_DELIM); - i += std::string(DIR_DELIM).size() - 1; // generally a no-op + i += strlen(DIR_DELIM) - 1; // generally a no-op } } @@ -259,6 +261,46 @@ void TestFileSys::testRemoveRelativePathComponent() } +void TestFileSys::testAbsolutePath() +{ + const auto dir_path = getTestTempDirectory(); + + /* AbsolutePath */ + UASSERTEQ(auto, fs::AbsolutePath(""), ""); // empty is a not valid path + const auto cwd = fs::AbsolutePath("."); + UASSERTCMP(auto, !=, cwd, ""); + { + const auto dir_path2 = getTestTempFile(); + UASSERTEQ(auto, fs::AbsolutePath(dir_path2), ""); // doesn't exist + fs::CreateDir(dir_path2); + UASSERTCMP(auto, !=, fs::AbsolutePath(dir_path2), ""); // now it does + UASSERTEQ(auto, fs::AbsolutePath(dir_path2 + DIR_DELIM ".."), fs::AbsolutePath(dir_path)); + } + + /* AbsolutePathPartial */ + // equivalent to AbsolutePath if it exists + UASSERTEQ(auto, fs::AbsolutePathPartial("."), cwd); + UASSERTEQ(auto, fs::AbsolutePathPartial(dir_path), fs::AbsolutePath(dir_path)); + // usual usage of the function with a partially existing path + auto expect = cwd + DIR_DELIM + p("does/not/exist"); + UASSERTEQ(auto, fs::AbsolutePathPartial("does/not/exist"), expect); + UASSERTEQ(auto, fs::AbsolutePathPartial(expect), expect); + + // a nonsense combination as you couldn't actually access it, but allowed by function + UASSERTEQ(auto, fs::AbsolutePathPartial("bla/blub/../.."), cwd); + UASSERTEQ(auto, fs::AbsolutePathPartial("./bla/blub/../.."), cwd); + +#ifdef __unix__ + // one way to produce the error case is to remove more components than there are + // but only if the path does not actually exist ("/.." does exist). + UASSERTEQ(auto, fs::AbsolutePathPartial("/.."), "/"); + UASSERTEQ(auto, fs::AbsolutePathPartial("/noexist/../.."), ""); +#endif + // or with an empty path + UASSERTEQ(auto, fs::AbsolutePathPartial(""), ""); +} + + void TestFileSys::testSafeWriteToFile() { const std::string dest_path = getTestTempFile();