Add path sanitization in virtfsdir.

5 files changed, 83 insertions, 35 deletions

diff --git a/src/fs/paths.cpp b/src/fs/paths.cpp
index aa1c453b0..8a9969461 100644
--- a/src/fs/paths.cpp
+++ b/src/fs/paths.cpp
@@ -27,6 +27,7 @@
 #include "fs/paths.h"
 #include "fs/virtfs.h"
 
+#include "utils/checkutils.h"
 #include "utils/stringutils.h"
 
 #ifdef USE_X11
@@ -101,6 +102,19 @@ bool checkPath(const std::string &path)
         && path.find("\\..") == std::string::npos;
 }
 
+void prepareFsPath(std::string &path)
+{
+    std::string path2 = path;
+    sanitizePath(path);
+// can be enabled for debugging
+//    if (path != path2)
+//    {
+//        reportAlways("Path can be improved: '%s' -> '%s'",
+//            path2.c_str(),
+//            path0.c_str());
+//    }
+}
+
 std::string &fixDirSeparators(std::string &str)
 {
     if (dirSeparator[0] == '/')
diff --git a/src/fs/paths.h b/src/fs/paths.h
index 48a6eeea8..be431f9c3 100644
--- a/src/fs/paths.h
+++ b/src/fs/paths.h
@@ -31,6 +31,8 @@ bool isRealPath(const std::string &str) A_WARN_UNUSED;
 
 bool checkPath(const std::string &path) A_WARN_UNUSED;
 
+void prepareFsPath(std::string &path);
+
 std::string &fixDirSeparators(std::string &str);
 
 std::string removeLast(const std::string &str) A_WARN_UNUSED;
diff --git a/src/fs/virtfsdir.cpp b/src/fs/virtfsdir.cpp
index 605ad2aac..6673072aa 100644
--- a/src/fs/virtfsdir.cpp
+++ b/src/fs/virtfsdir.cpp
@@ -60,7 +60,7 @@ namespace VirtFsDir
 {
     namespace
     {
-        static VirtFile *openFile(const std::string &restrict filename,
+        static VirtFile *openFile(std::string filename,
                                   const int mode)
         {
             if (checkPath(filename) == false)
@@ -69,6 +69,7 @@ namespace VirtFsDir
                     filename.c_str());
                 return nullptr;
             }
+            prepareFsPath(filename);
             VirtDirEntry *const entry = searchEntryByPath(filename);
             if (entry == nullptr)
                 return nullptr;
@@ -111,10 +112,11 @@ namespace VirtFsDir
         return nullptr;
     }
 
-    bool addToSearchPathSilent(const std::string &newDir,
+    bool addToSearchPathSilent(std::string newDir,
                                const Append append,
                                const SkipError skipError)
     {
+        prepareFsPath(newDir);
         if (skipError == SkipError_false &&
             Files::existsLocal(newDir) == false)
         {
@@ -152,9 +154,10 @@ namespace VirtFsDir
         return true;
     }
 
-    bool addToSearchPath(const std::string &newDir,
+    bool addToSearchPath(std::string newDir,
                          const Append append)
     {
+        prepareFsPath(newDir);
         if (Files::existsLocal(newDir) == false)
         {
             reportAlways("VirtFsDir::addToSearchPath directory not exists: %s",
@@ -193,6 +196,7 @@ namespace VirtFsDir
 
     bool removeFromSearchPathSilent(std::string oldDir)
     {
+        prepareFsPath(oldDir);
         if (oldDir.find(".zip") != std::string::npos)
         {
             reportAlways("Called removeFromSearchPath with zip archive");
@@ -219,6 +223,7 @@ namespace VirtFsDir
 
     bool removeFromSearchPath(std::string oldDir)
     {
+        prepareFsPath(oldDir);
         if (oldDir.find(".zip") != std::string::npos)
         {
             reportAlways("Called removeFromSearchPath with zip archive");
@@ -269,7 +274,9 @@ namespace VirtFsDir
         mBaseDir = getRealPath(getFileDir(name));
 #endif  // defined(__native_client__)
 
+        prepareFsPath(mBaseDir);
         mUserDir = getHomePath();
+        prepareFsPath(mUserDir);
         initFuncs(&funcs);
     }
 
@@ -294,8 +301,9 @@ namespace VirtFsDir
         return mUserDir.c_str();
     }
 
-    std::string getRealDir(const std::string &restrict filename)
+    std::string getRealDir(std::string filename)
     {
+        prepareFsPath(filename);
         if (checkPath(filename) == false)
         {
             reportAlways("VirtFsDir::exists invalid path: %s",
@@ -312,8 +320,9 @@ namespace VirtFsDir
         return std::string();
     }
 
-    bool exists(const std::string &restrict name)
+    bool exists(std::string name)
     {
+        prepareFsPath(name);
         if (checkPath(name) == false)
         {
             reportAlways("VirtFsDir::exists invalid path: %s",
@@ -329,8 +338,9 @@ namespace VirtFsDir
         return false;
     }
 
-    VirtList *enumerateFiles(const std::string &restrict dirName)
+    VirtList *enumerateFiles(std::string dirName)
     {
+        prepareFsPath(dirName);
         VirtList *const list = new VirtList;
         if (checkPath(dirName) == false)
         {
@@ -382,8 +392,9 @@ namespace VirtFsDir
         return list;
     }
 
-    bool isDirectory(const std::string &restrict dirName)
+    bool isDirectory(std::string dirName)
     {
+        prepareFsPath(dirName);
         if (checkPath(dirName) == false)
         {
             reportAlways("VirtFsDir::isDirectory invalid path: %s",
@@ -407,8 +418,9 @@ namespace VirtFsDir
         return false;
     }
 
-    bool isSymbolicLink(const std::string &restrict name)
+    bool isSymbolicLink(std::string name)
     {
+        prepareFsPath(name);
         if (checkPath(name) == false)
         {
             reportAlways("VirtFsDir::isSymbolicLink invalid path: %s",
@@ -443,16 +455,18 @@ namespace VirtFsDir
         return openFile(filename, O_WRONLY | O_CREAT | O_APPEND);
     }
 
-    bool setWriteDir(const std::string &restrict newDir)
+    bool setWriteDir(std::string newDir)
     {
+        prepareFsPath(newDir);
         mWriteDir = newDir;
         if (findLast(mWriteDir, std::string(dirSeparator)) == false)
             mWriteDir += dirSeparator;
         return true;
     }
 
-    bool mkdir(const std::string &restrict dirname)
+    bool mkdir(std::string dirname)
     {
+        prepareFsPath(dirname);
         if (mWriteDir.empty())
         {
             reportAlways("VirtFsDir::mkdir write dir is empty");
@@ -461,8 +475,9 @@ namespace VirtFsDir
         return mkdir_r((mWriteDir + dirname).c_str()) != -1;
     }
 
-    bool remove(const std::string &restrict filename)
+    bool remove(std::string filename)
     {
+        prepareFsPath(filename);
         if (mWriteDir.empty())
         {
             reportAlways("VirtFsDir::remove write dir is empty");
diff --git a/src/fs/virtfsdir.h b/src/fs/virtfsdir.h
index 0f36b1241..8ea825d82 100644
--- a/src/fs/virtfsdir.h
+++ b/src/fs/virtfsdir.h
@@ -40,9 +40,9 @@ namespace VirtFsDir
     VirtDirEntry *searchEntryByPath(const std::string &restrict path);
     const char *getBaseDir();
     const char *getUserDir();
-    bool addToSearchPath(const std::string &newDir,
+    bool addToSearchPath(std::string newDir,
                          const Append append);
-    bool addToSearchPathSilent(const std::string &newDir,
+    bool addToSearchPathSilent(std::string newDir,
                                const Append append,
                                const SkipError skipError);
     bool removeFromSearchPath(std::string oldDir);
@@ -51,18 +51,18 @@ namespace VirtFsDir
     void initFuncs(VirtFsFuncs *restrict const ptr);
     void deinit();
     std::vector<VirtDirEntry*> &getEntries();
-    bool exists(const std::string &restrict name);
-    VirtList *enumerateFiles(const std::string &restrict dir) RETURNS_NONNULL;
-    bool isDirectory(const std::string &restrict dirName);
-    bool isSymbolicLink(const std::string &restrict name);
+    bool exists(std::string name);
+    VirtList *enumerateFiles(std::string dir) RETURNS_NONNULL;
+    bool isDirectory(std::string dirName);
+    bool isSymbolicLink(std::string name);
     void freeList(VirtList *restrict const handle);
     VirtFile *openRead(const std::string &restrict filename);
     VirtFile *openWrite(const std::string &restrict filename);
     VirtFile *openAppend(const std::string &restrict filename);
-    bool setWriteDir(const std::string &restrict newDir);
-    std::string getRealDir(const std::string &restrict filename);
-    bool mkdir(const std::string &restrict dirName);
-    bool remove(const std::string &restrict filename);
+    bool setWriteDir(std::string newDir);
+    std::string getRealDir(std::string filename);
+    bool mkdir(std::string dirName);
+    bool remove(std::string filename);
     void permitLinks(const bool val);
     const char *getLastError();
     int64_t read(VirtFile *restrict const handle,
diff --git a/src/fs/virtfsdir_unittest.cc b/src/fs/virtfsdir_unittest.cc
index 1f162a6e9..589b10e4c 100644
--- a/src/fs/virtfsdir_unittest.cc
+++ b/src/fs/virtfsdir_unittest.cc
@@ -93,7 +93,7 @@ TEST_CASE("VirtFsDir addToSearchPath")
 
     SECTION("simple 4")
     {
-        REQUIRE(VirtFsDir::addToSearchPathSilent("dir1",
+        REQUIRE(VirtFsDir::addToSearchPathSilent("dir1\\",
             Append_true,
             SkipError_true));
         REQUIRE(VirtFsDir::addToSearchPathSilent("dir2",
@@ -105,7 +105,7 @@ TEST_CASE("VirtFsDir addToSearchPath")
         REQUIRE(VirtFsDir::getEntries().size() == 2);
         REQUIRE(VirtFsDir::getEntries()[0]->mRootDir == "dir1/");
         REQUIRE(VirtFsDir::getEntries()[1]->mRootDir == "dir2/");
-        REQUIRE(VirtFsDir::getEntries()[0]->mUserDir == "dir1");
+        REQUIRE(VirtFsDir::getEntries()[0]->mUserDir == "dir1/");
         REQUIRE(VirtFsDir::getEntries()[1]->mUserDir == "dir2");
     }
 
@@ -169,6 +169,7 @@ TEST_CASE("VirtFsDir removeFromSearchPath")
     SECTION("simple 1")
     {
         REQUIRE_THROWS(VirtFsDir::removeFromSearchPath("dir1"));
+        REQUIRE_THROWS(VirtFsDir::removeFromSearchPath("dir1/"));
     }
 
     SECTION("simple 2")
@@ -185,7 +186,7 @@ TEST_CASE("VirtFsDir removeFromSearchPath")
         REQUIRE(VirtFsDir::addToSearchPathSilent("dir1",
             Append_true,
             SkipError_true));
-        REQUIRE(VirtFsDir::addToSearchPathSilent("dir2/dir3",
+        REQUIRE(VirtFsDir::addToSearchPathSilent("dir2//dir3",
             Append_true,
             SkipError_true));
         REQUIRE(VirtFsDir::addToSearchPathSilent("dir3",
@@ -239,26 +240,26 @@ TEST_CASE("VirtFsDir exists")
 {
     VirtFsDir::init(".");
     logger = new Logger();
-    VirtFsDir::addToSearchPathSilent("data",
+    VirtFsDir::addToSearchPathSilent("data/",
         Append_false,
         SkipError_false);
-    VirtFsDir::addToSearchPathSilent("../data",
+    VirtFsDir::addToSearchPathSilent("..\\data",
         Append_false,
         SkipError_false);
 
-    REQUIRE(VirtFsDir::exists("test/units.xml") == true);
-    REQUIRE(VirtFsDir::exists("test/units123.xml") == false);
+    REQUIRE(VirtFsDir::exists("test//units.xml") == true);
+    REQUIRE(VirtFsDir::exists("test/\\units123.xml") == false);
     REQUIRE(VirtFsDir::exists("tesQ/units.xml") == false);
     REQUIRE(VirtFsDir::exists("units.xml") == false);
 
-    VirtFsDir::addToSearchPathSilent("data/test",
+    VirtFsDir::addToSearchPathSilent("data//test",
         Append_false,
         SkipError_false);
-    VirtFsDir::addToSearchPathSilent("../data/test",
+    VirtFsDir::addToSearchPathSilent("..//data\\test",
         Append_false,
         SkipError_false);
 
-    REQUIRE(VirtFsDir::exists("test/units.xml") == true);
+    REQUIRE(VirtFsDir::exists("test\\units.xml") == true);
     REQUIRE(VirtFsDir::exists("test/units123.xml") == false);
     REQUIRE(VirtFsDir::exists("tesQ/units.xml") == false);
     REQUIRE(VirtFsDir::exists("units.xml") == true);
@@ -266,7 +267,7 @@ TEST_CASE("VirtFsDir exists")
     VirtFsDir::removeFromSearchPathSilent("data/test");
     VirtFsDir::removeFromSearchPathSilent("../data/test");
 
-    REQUIRE(VirtFsDir::exists("test/units.xml") == true);
+    REQUIRE(VirtFsDir::exists("test\\units.xml") == true);
     REQUIRE(VirtFsDir::exists("test/units123.xml") == false);
     REQUIRE(VirtFsDir::exists("tesQ/units.xml") == false);
     REQUIRE(VirtFsDir::exists("units.xml") == false);
@@ -320,12 +321,20 @@ TEST_CASE("VirtFsDir getRealDir")
         REQUIRE(VirtFsDir::getRealDir("test") == "data");
         REQUIRE(VirtFsDir::getRealDir("test/test.txt") ==
             "data");
+        REQUIRE(VirtFsDir::getRealDir("test\\test.txt") ==
+            "data");
+        REQUIRE(VirtFsDir::getRealDir("test//test.txt") ==
+            "data");
     }
     else
     {
         REQUIRE(VirtFsDir::getRealDir("test") == "../data");
         REQUIRE(VirtFsDir::getRealDir("test/test.txt") ==
             "../data");
+        REQUIRE(VirtFsDir::getRealDir("test\\test.txt") ==
+            "../data");
+        REQUIRE(VirtFsDir::getRealDir("test//test.txt") ==
+            "../data");
     }
     REQUIRE(VirtFsDir::getRealDir("zzz") == "");
 
@@ -340,6 +349,8 @@ TEST_CASE("VirtFsDir getRealDir")
         REQUIRE(VirtFsDir::getRealDir("test") == "data");
         REQUIRE(VirtFsDir::getRealDir("test/test.txt") ==
             "data");
+        REQUIRE(VirtFsDir::getRealDir("test\\test.txt") ==
+            "data");
         REQUIRE(VirtFsDir::getRealDir("test.txt") ==
             "data/test");
     }
@@ -348,6 +359,8 @@ TEST_CASE("VirtFsDir getRealDir")
         REQUIRE(VirtFsDir::getRealDir("test") == "../data");
         REQUIRE(VirtFsDir::getRealDir("test/test.txt") ==
             "../data");
+        REQUIRE(VirtFsDir::getRealDir("test\\test.txt") ==
+            "../data");
         REQUIRE(VirtFsDir::getRealDir("test.txt") ==
             "../data/test");
     }
@@ -412,13 +425,13 @@ TEST_CASE("VirtFsDir enumerateFiles1")
     VirtFsDir::freeList(list);
 
     VirtFsDir::permitLinks(true);
-    list = VirtFsDir::enumerateFiles("test");
+    list = VirtFsDir::enumerateFiles("test/");
     removeTemp(list->names);
     REQUIRE(list->names.size() == cnt2);
     VirtFsDir::freeList(list);
 
     VirtFsDir::permitLinks(false);
-    list = VirtFsDir::enumerateFiles("test");
+    list = VirtFsDir::enumerateFiles("test\\");
     removeTemp(list->names);
     REQUIRE(list->names.size() == cnt1);
     VirtFsDir::freeList(list);
@@ -509,7 +522,7 @@ TEST_CASE("VirtFsDir isDirectory")
     REQUIRE(VirtFsDir::isDirectory("test//dir1") == true);
     REQUIRE(VirtFsDir::isDirectory("test//dir1/") == true);
     REQUIRE(VirtFsDir::isDirectory("test//dir1//") == true);
-    REQUIRE(VirtFsDir::isDirectory("test/dir1/") == true);
+    REQUIRE(VirtFsDir::isDirectory("test\\dir1/") == true);
     REQUIRE(VirtFsDir::isDirectory("test/dir1//") == true);
     REQUIRE(VirtFsDir::isDirectory("testQ") == false);
     REQUIRE(VirtFsDir::isDirectory("testQ/") == false);
@@ -531,6 +544,7 @@ TEST_CASE("VirtFsDir isDirectory")
     REQUIRE(VirtFsDir::isDirectory("test") == true);
     REQUIRE(VirtFsDir::isDirectory("testQ") == false);
     REQUIRE(VirtFsDir::isDirectory("test/dir1") == true);
+    REQUIRE(VirtFsDir::isDirectory("test\\dir1") == true);
 
     VirtFsDir::removeFromSearchPathSilent("data/test");
     VirtFsDir::removeFromSearchPathSilent("../data/test");
@@ -567,6 +581,9 @@ TEST_CASE("VirtFsDir openRead")
     file = VirtFsDir::openRead("test/units.xml");
     REQUIRE(file != nullptr);
     VirtFsDir::close(file);
+    file = VirtFsDir::openRead("test\\units.xml");
+    REQUIRE(file != nullptr);
+    VirtFsDir::close(file);
     file = VirtFsDir::openRead("test/units123.xml");
     REQUIRE(file == nullptr);
     file = VirtFsDir::openRead("tesQ/units.xml");