From 086c479a4cc77d247b83f964f0eabed812563526 Mon Sep 17 00:00:00 2001 From: Dimitry Sibiryakov Date: Sat, 30 Apr 2016 18:03:00 +0200 Subject: [PATCH 1/2] Fix CORE-5223 --- src/common/config/dir_list.cpp | 85 +++++++++++++++------------------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/src/common/config/dir_list.cpp b/src/common/config/dir_list.cpp index a5e95c1e46f..dad35192ed1 100644 --- a/src/common/config/dir_list.cpp +++ b/src/common/config/dir_list.cpp @@ -33,25 +33,44 @@ void ParsedPath::parse(const PathName& path) { clear(); - if (path.length() == 1) { - add(path); - return; - } - PathName oldpath = path; + int toSkip = 0; do { PathName newpath, elem; PathUtils::splitLastComponent(newpath, elem, oldpath); oldpath = newpath; + if (elem.isEmpty() && !oldpath.isEmpty()) // Skip double dir separator + { + continue; + } + if (elem == ".") // Skip current dir reference + { + continue; + } + if (elem == PathUtils::up_dir_link) // skip next up dir + { + toSkip++; + continue; + } + if (toSkip > 0) + { + toSkip--; + continue; + } insert(0, elem); } while (oldpath.length() > 0); + if (toSkip != 0) + { + // Malformed path, attempt to hack?.. + // Let it be, consequent comparison will rule it out + } } PathName ParsedPath::subPath(FB_SIZE_T n) const { PathName rc = (*this)[0]; - if (PathUtils::isRelative(rc + PathUtils::dir_sep)) - rc = PathUtils::dir_sep + rc; + if (rc.isEmpty()) + rc = PathUtils::dir_sep; for (FB_SIZE_T i = 1; i < n; i++) { PathName newpath; @@ -155,42 +174,23 @@ void DirectoryList::initialize(bool simple_mode) } } - FB_SIZE_T last = 0; PathName root = Config::getRootDirectory(); - FB_SIZE_T i; - for (i = 0; i < val.length(); i++) + while (!val.isEmpty()) { - if (val[i] == ';') + string::size_type sep = val.find(';'); + if (sep == string::npos) + sep = val.length(); + PathName dir(val.c_str(), sep); + dir.alltrim(" \t\r"); + val.erase(0, sep + 1); + if (PathUtils::isRelative(dir)) { - PathName dir = ""; - if (i > last) - { - dir = val.substr(last, i - last); - dir.trim(); - } - if (PathUtils::isRelative(dir)) - { - PathName newdir; - PathUtils::concatPath(newdir, root, dir); - dir = newdir; - } - add(ParsedPath(dir)); - last = i + 1; + PathName fullPath; + PathUtils::concatPath(fullPath, root, dir); + dir = fullPath; } + add(ParsedPath(dir)); } - PathName dir = ""; - if (i > last) - { - dir = val.substr(last, i - last); - dir.trim(); - } - if (PathUtils::isRelative(dir)) - { - PathName newdir; - PathUtils::concatPath(newdir, root, dir); - dir = newdir; - } - add(ParsedPath(dir)); } bool DirectoryList::isPathInList(const PathName& path) const @@ -211,15 +211,6 @@ bool DirectoryList::isPathInList(const PathName& path) const return true; } - // Disable any up-dir(..) references - in case our path_utils - // and OS handle paths in slightly different ways, - // this is "wonderful" potential hole for hacks - // Example of IIS attack attempt: - // "GET /scripts/..%252f../winnt/system32/cmd.exe?/c+dir HTTP/1.0" - // (live from apache access.log :) - if (path.find(PathUtils::up_dir_link) != PathName::npos) - return false; - PathName varpath(path); if (PathUtils::isRelative(path)) { PathUtils::concatPath(varpath, PathName(Config::getRootDirectory()), path); From 8967b64c2014d8052d03a11224883560d622576e Mon Sep 17 00:00:00 2001 From: Dimitry Sibiryakov Date: Fri, 6 May 2016 12:15:15 +0200 Subject: [PATCH 2/2] Use symbolic name for current dir --- src/common/config/config_file.cpp | 4 ++-- src/common/config/dir_list.cpp | 2 +- src/common/os/path_utils.h | 3 +++ src/common/os/posix/path_utils.cpp | 1 + src/common/os/win32/path_utils.cpp | 1 + 5 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/common/config/config_file.cpp b/src/common/config/config_file.cpp index f0c0963b316..209ba706e5a 100644 --- a/src/common/config/config_file.cpp +++ b/src/common/config/config_file.cpp @@ -746,7 +746,7 @@ bool ConfigFile::wildCards(const char* currentFileName, const PathName& pathPref // Any change in directory can cause config change PathName prefix(pathPrefix); if(!pathPrefix.hasData()) - prefix = "."; + prefix = PathUtils::curr_dir_link; bool found = false; PathName next(components.pop()); @@ -761,7 +761,7 @@ bool ConfigFile::wildCards(const char* currentFileName, const PathName& pathPref { PathName name; const PathName fileName = list.getFileName(); - if (fileName == ".") + if (fileName == PathUtils::curr_dir_link) continue; if (fileName[0] == '.' && next[0] != '.') continue; diff --git a/src/common/config/dir_list.cpp b/src/common/config/dir_list.cpp index dad35192ed1..b1dfa4bae00 100644 --- a/src/common/config/dir_list.cpp +++ b/src/common/config/dir_list.cpp @@ -43,7 +43,7 @@ void ParsedPath::parse(const PathName& path) { continue; } - if (elem == ".") // Skip current dir reference + if (elem == PathUtils::curr_dir_link) // Skip current dir reference { continue; } diff --git a/src/common/os/path_utils.h b/src/common/os/path_utils.h index 19b46af66ac..9d7ebeb0e03 100644 --- a/src/common/os/path_utils.h +++ b/src/common/os/path_utils.h @@ -45,6 +45,9 @@ class PathUtils /// The directory separator for the platform. static const char dir_sep; + /// String used to point to current directory + static const char* curr_dir_link; + /// String used to point to parent directory static const char* up_dir_link; diff --git a/src/common/os/posix/path_utils.cpp b/src/common/os/posix/path_utils.cpp index 36fdf01c5a5..12764853bcc 100644 --- a/src/common/os/posix/path_utils.cpp +++ b/src/common/os/posix/path_utils.cpp @@ -35,6 +35,7 @@ /// The POSIX implementation of the path_utils abstraction. const char PathUtils::dir_sep = '/'; +const char* PathUtils::curr_dir_link = "."; const char* PathUtils::up_dir_link = ".."; const char PathUtils::dir_list_sep = ':'; diff --git a/src/common/os/win32/path_utils.cpp b/src/common/os/win32/path_utils.cpp index 3584b072e04..1eca5a847b7 100644 --- a/src/common/os/win32/path_utils.cpp +++ b/src/common/os/win32/path_utils.cpp @@ -7,6 +7,7 @@ /// The Win32 implementation of the path_utils abstraction. const char PathUtils::dir_sep = '\\'; +const char* PathUtils::curr_dir_link = "."; const char* PathUtils::up_dir_link = ".."; const char PathUtils::dir_list_sep = ';';