diff --git a/ChangeLog b/ChangeLog index ecc44809..de70c0d2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2010-02-27 Tatsuhiro Tsujikawa + + Replaced null or control characters in file path with '_'. For + MinGW32 build, additional characters which is not allowed in + Windows kernel are also replaced. util::detectDirTraversal() now + returns true if given string contains null or control characters. + * src/DownloadContext.cc + * src/DownloadContext.h + * src/Metalink2RequestGroup.cc + * src/MetalinkParserController.cc + * src/bittorrent_helper.cc + * src/util.cc + * src/util.h + * test/UtilTest.cc + 2010-02-27 Tatsuhiro Tsujikawa Discard metalink:file if its name attribute is empty string. diff --git a/src/DownloadContext.cc b/src/DownloadContext.cc index 1460f237..a88fa1ee 100644 --- a/src/DownloadContext.cc +++ b/src/DownloadContext.cc @@ -38,6 +38,7 @@ #include "FileEntry.h" #include "StringFormat.h" +#include "util.h" namespace aria2 { @@ -59,7 +60,8 @@ DownloadContext::DownloadContext(size_t pieceLength, _downloadStartTime(0), _downloadStopTime(_downloadStartTime) { - SharedHandle fileEntry(new FileEntry(path, totalLength, 0)); + SharedHandle fileEntry + (new FileEntry(util::escapePath(path), totalLength, 0)); _fileEntries.push_back(fileEntry); } @@ -108,6 +110,7 @@ void DownloadContext::setFilePathWithIndex (size_t index, const std::string& path) { if(0 < index && index <= _fileEntries.size()) { + // We don't escape path because path may come from users. _fileEntries[index-1]->setPath(path); } else { throw DL_ABORT_EX(StringFormat("No such file with index=%u", diff --git a/src/DownloadContext.h b/src/DownloadContext.h index 271b1a0e..e2c5da42 100644 --- a/src/DownloadContext.h +++ b/src/DownloadContext.h @@ -216,10 +216,11 @@ public: void setFileFilter(IntSequence seq); - // Sets file path for specified index. index starts from 1. The index - // is the same used in setFileFilter(). Please note that path is - // not the actual file path. The actual file path is - // getDir()+"/"+path. + // Sets file path for specified index. index starts from 1. The + // index is the same used in setFileFilter(). Please note that path + // is not the actual file path. The actual file path is + // getDir()+"/"+path. path is not escaped by util::escapePath() in + // this function. void setFilePathWithIndex(size_t index, const std::string& path); void setAttribute(const std::string& key, const BDE& value); diff --git a/src/Metalink2RequestGroup.cc b/src/Metalink2RequestGroup.cc index 4ccd924e..e06e39aa 100644 --- a/src/Metalink2RequestGroup.cc +++ b/src/Metalink2RequestGroup.cc @@ -287,7 +287,8 @@ Metalink2RequestGroup::createRequestGroup AccumulateNonP2PUrl(uris)); SharedHandle fe (new FileEntry - (util::applyDir(option->get(PREF_DIR), (*i)->file->getPath()), + (util::applyDir(option->get(PREF_DIR), + util::escapePath((*i)->file->getPath())), (*i)->file->getLength(), offset, uris)); if(option->getAsBool(PREF_METALINK_ENABLE_UNIQUE_PROTOCOL)) { fe->disableSingleHostMultiConnection(); diff --git a/src/MetalinkParserController.cc b/src/MetalinkParserController.cc index d119bec7..f6d5ab81 100644 --- a/src/MetalinkParserController.cc +++ b/src/MetalinkParserController.cc @@ -86,7 +86,7 @@ void MetalinkParserController::setFileNameOfEntry(const std::string& filename) if(_tEntry->file.isNull()) { _tEntry->file.reset(new FileEntry(path, 0, 0)); } else { - _tEntry->file->setPath(path); + _tEntry->file->setPath(util::escapePath(path)); } } diff --git a/src/bittorrent_helper.cc b/src/bittorrent_helper.cc index 73bb434b..02b3d187 100644 --- a/src/bittorrent_helper.cc +++ b/src/bittorrent_helper.cc @@ -257,7 +257,7 @@ static void extractFileEntries std::deque uris; createUri(urlList.begin(), urlList.end(), std::back_inserter(uris), path); SharedHandle fileEntry - (new FileEntry(util::applyDir(ctx->getDir(), path), + (new FileEntry(util::applyDir(ctx->getDir(), util::escapePath(path)), fileLengthData.i(), offset, uris)); fileEntry->setOriginalName(path); @@ -287,7 +287,8 @@ static void extractFileEntries } SharedHandle fileEntry - (new FileEntry(util::applyDir(ctx->getDir(), name),totalLength, 0, + (new FileEntry(util::applyDir(ctx->getDir(), util::escapePath(name)), + totalLength, 0, uris)); fileEntry->setOriginalName(name); fileEntries.push_back(fileEntry); diff --git a/src/util.cc b/src/util.cc index 9139b529..2df4ff37 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1129,8 +1129,7 @@ std::string applyDir(const std::string& dir, const std::string& relPath) std::string fixTaintedBasename(const std::string& src) { - return replace(replace(src, A2STR::SLASH_C, A2STR::UNDERSCORE_C), - A2STR::BACK_SLASH_C, A2STR::UNDERSCORE_C); + return escapePath(replace(src, A2STR::SLASH_C, A2STR::UNDERSCORE_C)); } void generateRandomKey(unsigned char* key) @@ -1169,6 +1168,11 @@ bool inPrivateAddress(const std::string& ipv4addr) bool detectDirTraversal(const std::string& s) { + for(std::string::const_iterator i = s.begin(); i != s.end(); ++i) { + if(0x00 <= (*i) && (*i) <= 0x1f) { + return true; + } + } return s == A2STR::DOT_C || s == ".." || util::startsWith(s, A2STR::SLASH_C) || @@ -1181,6 +1185,40 @@ bool detectDirTraversal(const std::string& s) util::endsWith(s, "/.."); } +namespace { +class EscapePath { +private: + char _repChar; +public: + EscapePath(const char& repChar):_repChar(repChar) {} + + char operator()(const char& c) { + if(0x00 <= c && c <=0x1f) { + return _repChar; + } +#ifdef __MINGW32__ + // We don't escape '/' because we use it as a path separator. + static const char WIN_INVALID_PATH_CHARS[] = + { '"', '*', ':', '<', '>', '?', '\\', '|' }; + if(std::find(&WIN_INVALID_PATH_CHARS[0], + &WIN_INVALID_PATH_CHARS[arrayLength(WIN_INVALID_PATH_CHARS)], + c) != + &WIN_INVALID_PATH_CHARS[arrayLength(WIN_INVALID_PATH_CHARS)]) { + return _repChar; + } +#endif // __MINGW32__ + return c; + } +}; +} + +std::string escapePath(const std::string& s) +{ + std::string d = s; + std::transform(d.begin(), d.end(), d.begin(), EscapePath('_')); + return d; +} + } // namespace util } // namespace aria2 diff --git a/src/util.h b/src/util.h index d9877784..e7b83e56 100644 --- a/src/util.h +++ b/src/util.h @@ -371,7 +371,8 @@ std::string applyDir(const std::string& dir, const std::string& relPath); // basename contains dir component. This should be avoided. This // function is created to fix these issues. This function expects src // should be non-percent-encoded basename. Currently, this function -// replaces '/' and '\' with '_'. +// replaces '/' with '_' and result string is passed to escapePath() +// function and its result is returned. std::string fixTaintedBasename(const std::string& src); // Generates 20 bytes random key and store it to the address pointed @@ -382,9 +383,15 @@ void generateRandomKey(unsigned char* key); bool inPrivateAddress(const std::string& ipv4addr); // Returns true if s contains directory traversal path component such -// as '..'. +// as '..' or it contains null or control character which may fool +// user. bool detectDirTraversal(const std::string& s); +// Replaces null(0x00) and control character(0x01-0x1f) with '_'. If +// __MINGW32__ is defined, following characters are also replaced with +// '_': '"', '*', ':', '<', '>', '?', '\', '|'. +std::string escapePath(const std::string& s); + } // namespace util } // namespace aria2 diff --git a/test/UtilTest.cc b/test/UtilTest.cc index 99b4b97d..91ab1355 100644 --- a/test/UtilTest.cc +++ b/test/UtilTest.cc @@ -62,6 +62,7 @@ class UtilTest:public CppUnit::TestFixture { CPPUNIT_TEST(testFixTaintedBasename); CPPUNIT_TEST(testIsNumericHost); CPPUNIT_TEST(testDetectDirTraversal); + CPPUNIT_TEST(testEscapePath); CPPUNIT_TEST_SUITE_END(); private: @@ -112,6 +113,7 @@ public: void testFixTaintedBasename(); void testIsNumericHost(); void testDetectDirTraversal(); + void testEscapePath(); }; @@ -1006,8 +1008,11 @@ void UtilTest::testApplyDir() void UtilTest::testFixTaintedBasename() { CPPUNIT_ASSERT_EQUAL(std::string("a_b"), util::fixTaintedBasename("a/b")); +#ifdef __MINGW32__ CPPUNIT_ASSERT_EQUAL(std::string("a_b"), util::fixTaintedBasename("a\\b")); - CPPUNIT_ASSERT_EQUAL(std::string("a__b"), util::fixTaintedBasename("a\\/b")); +#else // !__MINGW32__ + CPPUNIT_ASSERT_EQUAL(std::string("a\\b"), util::fixTaintedBasename("a\\b")); +#endif // !__MINGW32__ } void UtilTest::testIsNumericHost() @@ -1030,8 +1035,22 @@ void UtilTest::testDetectDirTraversal() CPPUNIT_ASSERT(util::detectDirTraversal("..")); CPPUNIT_ASSERT(util::detectDirTraversal("/")); CPPUNIT_ASSERT(util::detectDirTraversal("foo/")); + CPPUNIT_ASSERT(util::detectDirTraversal("\t")); CPPUNIT_ASSERT(!util::detectDirTraversal("foo/bar")); CPPUNIT_ASSERT(!util::detectDirTraversal("foo")); } +void UtilTest::testEscapePath() +{ + CPPUNIT_ASSERT_EQUAL(std::string("foo_bar__"), + util::escapePath(std::string("foo")+(char)0x00+ + std::string("bar")+(char)0x00+ + (char)0x01)); +#ifdef __MINGW32__ + CPPUNIT_ASSERT_EQUAL(std::string("foo_bar"), util::escapePath("foo\\bar")); +#else // !__MINGW32__ + CPPUNIT_ASSERT_EQUAL(std::string("foo\\bar"), util::escapePath("foo\\bar")); +#endif // !__MINGW32__ +} + } // namespace aria2