2010-02-27 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	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

ChangeLog

@@ -1,3 +1,18 @@
+2010-02-27  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	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  <t-tujikawa@users.sourceforge.net>
 
 	Discard metalink:file if its name attribute is empty string.

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> fileEntry(new FileEntry(path, totalLength, 0));
+  SharedHandle<FileEntry> 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",

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

src/Metalink2RequestGroup.cc

@@ -287,7 +287,8 @@ Metalink2RequestGroup::createRequestGroup
                       AccumulateNonP2PUrl(uris));
         SharedHandle<FileEntry> 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();

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

src/bittorrent_helper.cc

@@ -257,7 +257,7 @@ static void extractFileEntries
       std::deque<std::string> uris;
       createUri(urlList.begin(), urlList.end(), std::back_inserter(uris), path);
       SharedHandle<FileEntry> 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> 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);

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

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

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