From 4b18114e31ea277f9961ea279579da4157ecc2d7 Mon Sep 17 00:00:00 2001 From: Andrei Karas Date: Tue, 28 Feb 2017 01:31:24 +0300 Subject: Add path sanitization in virtfszip and zip. --- src/fs/virtfszip.cpp | 22 ++++++--- src/fs/virtfszip.h | 12 ++--- src/fs/virtfszip_unittest.cc | 108 +++++++++++++++++++++++-------------------- src/fs/zip.cpp | 2 + 4 files changed, 83 insertions(+), 61 deletions(-) (limited to 'src') diff --git a/src/fs/virtfszip.cpp b/src/fs/virtfszip.cpp index fa47892b5..35b94c780 100644 --- a/src/fs/virtfszip.cpp +++ b/src/fs/virtfszip.cpp @@ -72,9 +72,10 @@ namespace VirtFsZip return nullptr; } - bool addToSearchPathSilent(const std::string &newDir, + bool addToSearchPathSilent(std::string newDir, const Append append) { + prepareFsPath(newDir); if (Files::existsLocal(newDir) == false) { logger->log("VirtFsZip::addToSearchPath file not exists: %s", @@ -112,9 +113,10 @@ namespace VirtFsZip return true; } - bool addToSearchPath(const std::string &newDir, + bool addToSearchPath(std::string newDir, const Append append) { + prepareFsPath(newDir); if (Files::existsLocal(newDir) == false) { reportAlways("VirtFsZip::addToSearchPath directory not exists: %s", @@ -154,6 +156,7 @@ namespace VirtFsZip bool removeFromSearchPathSilent(std::string oldDir) { + prepareFsPath(oldDir); if (findLast(oldDir, ".zip") == false) { reportAlways("Called removeFromSearchPath without zip archive"); @@ -178,6 +181,7 @@ namespace VirtFsZip bool removeFromSearchPath(std::string oldDir) { + prepareFsPath(oldDir); if (findLast(oldDir, ".zip") == false) { reportAlways("Called removeFromSearchPath without zip archive"); @@ -227,8 +231,9 @@ namespace VirtFsZip ptr->eof = &VirtFsZip::eof; } - std::string getRealDir(const std::string &restrict filename) + std::string getRealDir(std::string filename) { + prepareFsPath(filename); if (checkPath(filename) == false) { reportAlways("VirtFsZip::exists invalid path: %s", @@ -241,8 +246,9 @@ namespace VirtFsZip return std::string(); } - bool exists(const std::string &restrict name) + bool exists(std::string name) { + prepareFsPath(name); if (checkPath(name) == false) { reportAlways("VirtFsZip::exists invalid path: %s", @@ -257,6 +263,7 @@ namespace VirtFsZip VirtList *enumerateFiles(std::string dirName) { + prepareFsPath(dirName); VirtList *const list = new VirtList; if (checkPath(dirName) == false) { @@ -302,6 +309,7 @@ namespace VirtFsZip bool isDirectory(std::string dirName) { + prepareFsPath(dirName); if (checkPath(dirName) == false) { reportAlways("VirtFsZip::isDirectory invalid path: %s", @@ -324,8 +332,9 @@ namespace VirtFsZip return false; } - bool isSymbolicLink(const std::string &restrict name) + bool isSymbolicLink(std::string name) { + prepareFsPath(name); if (checkPath(name) == false) { reportAlways("VirtFsZip::isSymbolicLink invalid path: %s", @@ -341,8 +350,9 @@ namespace VirtFsZip delete handle; } - VirtFile *openRead(const std::string &restrict filename) + VirtFile *openRead(std::string filename) { + prepareFsPath(filename); if (checkPath(filename) == false) { reportAlways("VirtFsZip::openRead invalid path: %s", diff --git a/src/fs/virtfszip.h b/src/fs/virtfszip.h index 81feb3693..d66ce7ee8 100644 --- a/src/fs/virtfszip.h +++ b/src/fs/virtfszip.h @@ -39,9 +39,9 @@ namespace VirtFsZip { VirtZipEntry *searchEntryByArchive(const std::string &restrict archiveName); ZipLocalHeader *searchHeaderByName(const std::string &restrict filename); - 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); bool removeFromSearchPath(std::string oldDir); bool removeFromSearchPathSilent(std::string oldDir); @@ -49,16 +49,16 @@ namespace VirtFsZip void initFuncs(VirtFsFuncs *restrict const ptr); void deinit(); std::vector &getEntries(); - bool exists(const std::string &restrict name); + bool exists(std::string name); VirtList *enumerateFiles(std::string dir) RETURNS_NONNULL; bool isDirectory(std::string dirName); - bool isSymbolicLink(const std::string &restrict name); + bool isSymbolicLink(std::string name); void freeList(VirtList *restrict const handle); - VirtFile *openRead(const std::string &restrict filename); + VirtFile *openRead(std::string 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); + std::string getRealDir(std::string filename); bool mkdir(const std::string &restrict dirName); bool remove(const std::string &restrict filename); void permitLinks(const bool val); diff --git a/src/fs/virtfszip_unittest.cc b/src/fs/virtfszip_unittest.cc index 05167a358..3eb8a2357 100644 --- a/src/fs/virtfszip_unittest.cc +++ b/src/fs/virtfszip_unittest.cc @@ -50,7 +50,7 @@ TEST_CASE("VirtFsZip addToSearchPath") SECTION("simple 1") { - REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "test.zip", + REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "/test.zip", Append_false)); REQUIRE(VirtFsZip::searchEntryByArchive( prefix + "test.zip") != nullptr); @@ -63,7 +63,7 @@ TEST_CASE("VirtFsZip addToSearchPath") SECTION("simple 2") { - REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "test.zip", + REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "\\test.zip", Append_true)); REQUIRE(VirtFsZip::searchEntryByArchive( prefix + "test.zip") != nullptr); @@ -212,7 +212,7 @@ TEST_CASE("VirtFsZip removeFromSearchPath") prefix + "test3.zip"); REQUIRE(VirtFsZip::getEntries()[1]->mArchiveName == prefix + "test2.zip"); - REQUIRE(VirtFsZip::removeFromSearchPath(prefix + "test2.zip")); + REQUIRE(VirtFsZip::removeFromSearchPath(prefix + "//test2.zip")); REQUIRE_THROWS(VirtFsZip::removeFromSearchPath(prefix + "test2.zip")); REQUIRE(VirtFsZip::getEntries().size() == 1); REQUIRE(VirtFsZip::getEntries()[0]->mArchiveName == @@ -221,13 +221,13 @@ TEST_CASE("VirtFsZip removeFromSearchPath") SECTION("simple 4") { - REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "test.zip", + REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "\\test.zip", Append_true)); REQUIRE(VirtFsZip::getEntries().size() == 1); REQUIRE(VirtFsZip::getEntries()[0]->mArchiveName == prefix + "test.zip"); REQUIRE_THROWS(VirtFsZip::removeFromSearchPath(prefix + "test2.zip")); - REQUIRE(VirtFsZip::removeFromSearchPath(prefix + "test.zip")); + REQUIRE(VirtFsZip::removeFromSearchPath(prefix + "\\test.zip")); REQUIRE(VirtFsZip::getEntries().size() == 0); REQUIRE(VirtFsZip::addToSearchPathSilent(prefix + "test.zip", Append_true)); @@ -244,12 +244,12 @@ TEST_CASE("VirtFsZip exists") { VirtFsZip::init(); logger = new Logger(); - VirtFsZip::addToSearchPathSilent("data/test/test2.zip", + VirtFsZip::addToSearchPathSilent("data\\test/test2.zip", Append_false); - VirtFsZip::addToSearchPathSilent("../data/test/test2.zip", + VirtFsZip::addToSearchPathSilent("../data\\test/test2.zip", Append_false); - REQUIRE(VirtFsZip::exists("dir2/units.xml") == true); + REQUIRE(VirtFsZip::exists("dir2//units.xml") == true); REQUIRE(VirtFsZip::exists("test/units123.xml") == false); REQUIRE(VirtFsZip::exists("tesQ/units.xml") == false); REQUIRE(VirtFsZip::exists("units1.xml") == false); @@ -261,7 +261,7 @@ TEST_CASE("VirtFsZip exists") VirtFsZip::addToSearchPathSilent("../data/test/test.zip", Append_false); - REQUIRE(VirtFsZip::exists("dir2/units.xml") == true); + REQUIRE(VirtFsZip::exists("dir2\\units.xml") == true); REQUIRE(VirtFsZip::exists("test/units123.xml") == false); REQUIRE(VirtFsZip::exists("tesQ/units.xml") == false); REQUIRE(VirtFsZip::exists("units1.xml") == false); @@ -271,11 +271,11 @@ TEST_CASE("VirtFsZip exists") VirtFsZip::removeFromSearchPathSilent("data/test/test2.zip"); VirtFsZip::removeFromSearchPathSilent("../data/test/test2.zip"); - REQUIRE(VirtFsZip::exists("dir2/units.xml") == false); + REQUIRE(VirtFsZip::exists("dir2//units.xml") == false); REQUIRE(VirtFsZip::exists("test/units123.xml") == false); REQUIRE(VirtFsZip::exists("tesQ/units.xml") == false); REQUIRE(VirtFsZip::exists("units1.xml") == false); - REQUIRE(VirtFsZip::exists("dir/hide.png") == true); + REQUIRE(VirtFsZip::exists("dir/\\/hide.png") == true); REQUIRE(VirtFsZip::exists("dir/brimmedhat.png") == true); REQUIRE_THROWS(VirtFsZip::exists("test/../units.xml")); @@ -298,17 +298,17 @@ TEST_CASE("VirtFsZip getRealDir") REQUIRE(VirtFsZip::getRealDir(".") == ""); REQUIRE(VirtFsZip::getRealDir("..") == ""); REQUIRE(VirtFsZip::getRealDir("test.txt") == prefix + "test2.zip"); - REQUIRE(VirtFsZip::getRealDir("dir/dye.png") == + REQUIRE(VirtFsZip::getRealDir("dir\\dye.png") == prefix + "test2.zip"); REQUIRE(VirtFsZip::getRealDir("zzz") == ""); VirtFsZip::addToSearchPathSilent(prefix + "test.zip", Append_false); - REQUIRE(VirtFsZip::getRealDir("dir/dye.png") == + REQUIRE(VirtFsZip::getRealDir("dir//dye.png") == prefix + "test2.zip"); - REQUIRE(VirtFsZip::getRealDir("dir/hide.png") == + REQUIRE(VirtFsZip::getRealDir("dir///hide.png") == prefix + "test.zip"); - REQUIRE(VirtFsZip::getRealDir("dir/brimmedhat.png") == + REQUIRE(VirtFsZip::getRealDir("dir\\\\brimmedhat.png") == prefix + "test.zip"); REQUIRE(VirtFsZip::getRealDir("zzz") == ""); @@ -316,7 +316,7 @@ TEST_CASE("VirtFsZip getRealDir") REQUIRE(VirtFsZip::getRealDir("dir/brimmedhat.png") == ""); REQUIRE(VirtFsZip::getRealDir("test.txt") == prefix + "test2.zip"); - REQUIRE(VirtFsZip::getRealDir("dir/dye.png") == + REQUIRE(VirtFsZip::getRealDir("dir//dye.png") == prefix + "test2.zip"); REQUIRE(VirtFsZip::getRealDir("zzz") == ""); @@ -341,7 +341,7 @@ TEST_CASE("VirtFsZip enumerateFiles1") VirtFsZip::init(); logger = new Logger; std::string name("data/test/test.zip"); - std::string prefix("data/test/"); + std::string prefix("data\\test/"); if (Files::existsLocal(name) == false) prefix = "../" + prefix; @@ -366,7 +366,7 @@ TEST_CASE("VirtFsZip enumerateFiles2") VirtFsZip::init(); logger = new Logger; std::string name("data/test/test.zip"); - std::string prefix("data/test/"); + std::string prefix("data//test/"); if (Files::existsLocal(name) == false) prefix = "../" + prefix; @@ -409,74 +409,73 @@ TEST_CASE("VirtFsZip isDirectory") VirtFsZip::addToSearchPathSilent(prefix + "test2.zip", Append_false); -// +++ need uncomment this lines after path sanitization will be added REQUIRE(VirtFsZip::isDirectory("dir2/units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units.xml/") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units123.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); REQUIRE(VirtFsZip::isDirectory("tesQ/units.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); REQUIRE(VirtFsZip::isDirectory("units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir") == true); -// REQUIRE(VirtFsZip::isDirectory("dir2/") == true); -// REQUIRE(VirtFsZip::isDirectory("dir2//") == true); + REQUIRE(VirtFsZip::isDirectory("dir2/") == true); + REQUIRE(VirtFsZip::isDirectory("dir2//") == true); REQUIRE(VirtFsZip::isDirectory("dir/1") == true); -// REQUIRE(VirtFsZip::isDirectory("dir//1") == true); -// REQUIRE(VirtFsZip::isDirectory("dir//1/") == true); + REQUIRE(VirtFsZip::isDirectory("dir//1") == true); + REQUIRE(VirtFsZip::isDirectory("dir\\1/") == true); REQUIRE(VirtFsZip::isDirectory("dir/1") == true); REQUIRE(VirtFsZip::isDirectory("dir/1/zzz") == false); -// REQUIRE(VirtFsZip::isDirectory("test/dir1//") == false); + REQUIRE(VirtFsZip::isDirectory("test/dir1\\") == false); REQUIRE(VirtFsZip::isDirectory("testQ") == false); REQUIRE(VirtFsZip::isDirectory("testQ/") == false); -// REQUIRE(VirtFsZip::isDirectory("testQ//") == false); + REQUIRE(VirtFsZip::isDirectory("testQ//") == false); VirtFsZip::addToSearchPathSilent(prefix + "test.zip", Append_false); REQUIRE(VirtFsZip::isDirectory("dir2/units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units.xml/") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2\\units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units123.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); REQUIRE(VirtFsZip::isDirectory("tesQ/units.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); REQUIRE(VirtFsZip::isDirectory("units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir") == true); REQUIRE(VirtFsZip::isDirectory("dir2/") == true); -// REQUIRE(VirtFsZip::isDirectory("dir2//") == true); + REQUIRE(VirtFsZip::isDirectory("dir2\\") == true); REQUIRE(VirtFsZip::isDirectory("dir/1") == true); -// REQUIRE(VirtFsZip::isDirectory("dir//1") == true); -// REQUIRE(VirtFsZip::isDirectory("dir//1/") == true); + REQUIRE(VirtFsZip::isDirectory("dir//1") == true); + REQUIRE(VirtFsZip::isDirectory("dir//1/") == true); REQUIRE(VirtFsZip::isDirectory("dir/1") == true); REQUIRE(VirtFsZip::isDirectory("dir/1/zzz") == false); -// REQUIRE(VirtFsZip::isDirectory("test/dir1//") == false); + REQUIRE(VirtFsZip::isDirectory("test/dir1//") == false); REQUIRE(VirtFsZip::isDirectory("testQ") == false); REQUIRE(VirtFsZip::isDirectory("testQ/") == false); -// REQUIRE(VirtFsZip::isDirectory("testQ//") == false); + REQUIRE(VirtFsZip::isDirectory("testQ//") == false); VirtFsZip::removeFromSearchPathSilent(prefix + "test2.zip"); REQUIRE(VirtFsZip::isDirectory("dir2/units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units.xml/") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir2/units123.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//units123.xml") == false); REQUIRE(VirtFsZip::isDirectory("tesQ/units.xml") == false); -// REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); + REQUIRE(VirtFsZip::isDirectory("tesQ//units.xml") == false); REQUIRE(VirtFsZip::isDirectory("units.xml") == false); REQUIRE(VirtFsZip::isDirectory("dir") == true); REQUIRE(VirtFsZip::isDirectory("dir2/") == false); -// REQUIRE(VirtFsZip::isDirectory("dir2//") == false); + REQUIRE(VirtFsZip::isDirectory("dir2//") == false); REQUIRE(VirtFsZip::isDirectory("dir/1") == false); -// REQUIRE(VirtFsZip::isDirectory("dir//1") == false); -// REQUIRE(VirtFsZip::isDirectory("dir//1/") == false); + REQUIRE(VirtFsZip::isDirectory("dir\\1") == false); + REQUIRE(VirtFsZip::isDirectory("dir//1/") == false); REQUIRE(VirtFsZip::isDirectory("dir/1") == false); REQUIRE(VirtFsZip::isDirectory("dir/1/zzz") == false); -// REQUIRE(VirtFsZip::isDirectory("test/dir1//") == false); + REQUIRE(VirtFsZip::isDirectory("test/dir1//") == false); REQUIRE(VirtFsZip::isDirectory("testQ") == false); REQUIRE(VirtFsZip::isDirectory("testQ/") == false); -// REQUIRE(VirtFsZip::isDirectory("testQ//") == false); + REQUIRE(VirtFsZip::isDirectory("testQ//") == false); VirtFsZip::removeFromSearchPathSilent(prefix + "test.zip"); VirtFsZip::deinit(); @@ -500,6 +499,9 @@ TEST_CASE("VirtFsZip openRead") file = VirtFsZip::openRead("dir2/units.xml"); REQUIRE(file != nullptr); VirtFsZip::close(file); + file = VirtFsZip::openRead("dir2\\units.xml"); + REQUIRE(file != nullptr); + VirtFsZip::close(file); file = VirtFsZip::openRead("dir2/units123.xml"); REQUIRE(file == nullptr); file = VirtFsZip::openRead("tesQ/units.xml"); @@ -510,6 +512,8 @@ TEST_CASE("VirtFsZip openRead") REQUIRE(file == nullptr); file = VirtFsZip::openRead("dir/brimmedhat.png"); REQUIRE(file == nullptr); + file = VirtFsZip::openRead("dir//brimmedhat.png"); + REQUIRE(file == nullptr); VirtFsZip::addToSearchPathSilent(prefix + "test.zip", Append_false); @@ -517,6 +521,9 @@ TEST_CASE("VirtFsZip openRead") file = VirtFsZip::openRead("dir2/units.xml"); REQUIRE(file != nullptr); VirtFsZip::close(file); + file = VirtFsZip::openRead("dir2//units.xml"); + REQUIRE(file != nullptr); + VirtFsZip::close(file); file = VirtFsZip::openRead("dir2/units123.xml"); REQUIRE(file == nullptr); file = VirtFsZip::openRead("tesQ/units.xml"); @@ -534,6 +541,9 @@ TEST_CASE("VirtFsZip openRead") file = VirtFsZip::openRead("dir2/units.xml"); REQUIRE(file != nullptr); VirtFsZip::close(file); + file = VirtFsZip::openRead("dir2\\/\\units.xml"); + REQUIRE(file != nullptr); + VirtFsZip::close(file); file = VirtFsZip::openRead("dir2/units123.xml"); REQUIRE(file == nullptr); file = VirtFsZip::openRead("tesQ/units.xml"); @@ -567,7 +577,7 @@ TEST_CASE("VirtFsZip read") SECTION("test 1") { - file = VirtFsZip::openRead("dir2/test.txt"); + file = VirtFsZip::openRead("dir2//test.txt"); REQUIRE(file != nullptr); REQUIRE(VirtFsZip::fileLength(file) == 23); const int fileSize = VirtFsZip::fileLength(file); @@ -582,7 +592,7 @@ TEST_CASE("VirtFsZip read") SECTION("test 2") { - file = VirtFsZip::openRead("dir2/test.txt"); + file = VirtFsZip::openRead("dir2\\/test.txt"); REQUIRE(file != nullptr); REQUIRE(VirtFsZip::fileLength(file) == 23); const int fileSize = VirtFsZip::fileLength(file); @@ -599,7 +609,7 @@ TEST_CASE("VirtFsZip read") SECTION("test 3") { - file = VirtFsZip::openRead("dir2/test.txt"); + file = VirtFsZip::openRead("dir2//test.txt"); REQUIRE(file != nullptr); const int fileSize = VirtFsZip::fileLength(file); @@ -634,7 +644,7 @@ TEST_CASE("VirtFsZip read") SECTION("test 5") { - file = VirtFsZip::openRead("dir2/test.txt"); + file = VirtFsZip::openRead("dir2\\\\test.txt"); REQUIRE(file != nullptr); const int fileSize = VirtFsZip::fileLength(file); const char *restrict const str = "test line 1\ntest line 2"; @@ -655,7 +665,7 @@ TEST_CASE("VirtFsZip read") SECTION("test 6") { - file = VirtFsZip::openRead("dir2/test.txt"); + file = VirtFsZip::openRead("dir2//test.txt"); REQUIRE(file != nullptr); const int fileSize = VirtFsZip::fileLength(file); const char *restrict const str = "test line 1\ntest line 2"; diff --git a/src/fs/zip.cpp b/src/fs/zip.cpp index 62b60a451..1bdc22639 100644 --- a/src/fs/zip.cpp +++ b/src/fs/zip.cpp @@ -20,6 +20,7 @@ #include "fs/zip.h" +#include "fs/paths.h" #include "fs/virtzipentry.h" #include "fs/ziplocalheader.h" @@ -123,6 +124,7 @@ namespace Zip buf[fileNameLen] = 0; header->fileName = std::string( reinterpret_cast(buf)); + prepareFsPath(header->fileName); header->dataOffset = ftell(arcFile) + extraFieldLen; fseek(arcFile, extraFieldLen + header->compressSize, SEEK_CUR); // pointer on 30 + fileNameLen + extraFieldLen + compressSize -- cgit v1.2.3-60-g2f50