BRNSystems/minetest
1
0
Fork 0
forked from Mirrorlandia_minetest/minetest
Code Pull Requests 1 Activity

Security: Fix resolving of some relative paths

Browse Source 
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`).
This commit is contained in:
ShadowNinja 2016-12-16 17:43:39 -05:00 committed by Craig Robbins
parent f522e7351a
commit 0f0502109e
1 changed files with 18 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
src/script/cpp_api/s_security.cpp
View File

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