diff options
-rw-r--r-- | base/file_util.cc | 12 | ||||
-rw-r--r-- | base/file_util.h | 12 | ||||
-rw-r--r-- | base/file_util_mac.mm | 2 | ||||
-rw-r--r-- | base/file_util_posix.cc | 4 | ||||
-rw-r--r-- | base/file_util_unittest.cc | 18 | ||||
-rw-r--r-- | base/file_util_win.cc | 4 |
6 files changed, 44 insertions, 8 deletions
diff --git a/base/file_util.cc b/base/file_util.cc index 7efb22b..07f2771 100644 --- a/base/file_util.cc +++ b/base/file_util.cc @@ -77,6 +77,18 @@ void InsertBeforeExtension(FilePath* path, const FilePath::StringType& suffix) { value.insert(last_dot, suffix); } +bool Move(const FilePath& from_path, const FilePath& to_path) { + if (from_path.ReferencesParent() || to_path.ReferencesParent()) + return false; + return MoveUnsafe(from_path, to_path); +} + +bool CopyFile(const FilePath& from_path, const FilePath& to_path) { + if (from_path.ReferencesParent() || to_path.ReferencesParent()) + return false; + return CopyFileUnsafe(from_path, to_path); +} + bool ContentsEqual(const FilePath& filename1, const FilePath& filename2) { // We open the file in binary format even if they are text files because // we are just comparing that bytes are exactly same in both files and not diff --git a/base/file_util.h b/base/file_util.h index b450087..254f326 100644 --- a/base/file_util.h +++ b/base/file_util.h @@ -120,9 +120,15 @@ BASE_EXPORT bool DeleteAfterReboot(const base::FilePath& path); // If a simple rename is not possible, such as in the case where the paths are // on different volumes, this will attempt to copy and delete. Returns // true for success. +// This function fails if either path contains traversal components ('..'). BASE_EXPORT bool Move(const base::FilePath& from_path, const base::FilePath& to_path); +// Same as Move but allows paths with traversal components. +// Use only with extreme care. +BASE_EXPORT bool MoveUnsafe(const base::FilePath& from_path, + const base::FilePath& to_path); + // Renames file |from_path| to |to_path|. Both paths must be on the same // volume, or the function will fail. Destination file will be created // if it doesn't exist. Prefer this function over Move when dealing with @@ -132,9 +138,15 @@ BASE_EXPORT bool ReplaceFile(const base::FilePath& from_path, const base::FilePath& to_path); // Copies a single file. Use CopyDirectory to copy directories. +// This function fails if either path contains traversal components ('..'). BASE_EXPORT bool CopyFile(const base::FilePath& from_path, const base::FilePath& to_path); +// Same as CopyFile but allows paths with traversal components. +// Use only with extreme care. +BASE_EXPORT bool CopyFileUnsafe(const base::FilePath& from_path, + const base::FilePath& to_path); + // Copies the given path, and optionally all subdirectories and their contents // as well. // If there are files existing under to_path, always overwrite. diff --git a/base/file_util_mac.mm b/base/file_util_mac.mm index 0638167..2499f44 100644 --- a/base/file_util_mac.mm +++ b/base/file_util_mac.mm @@ -27,7 +27,7 @@ bool GetShmemTempDir(FilePath* path, bool executable) { return GetTempDir(path); } -bool CopyFile(const FilePath& from_path, const FilePath& to_path) { +bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { base::ThreadRestrictions::AssertIOAllowed(); return (copyfile(from_path.value().c_str(), to_path.value().c_str(), NULL, COPYFILE_ALL) == 0); diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc index 49cf873..ebe3fbf 100644 --- a/base/file_util_posix.cc +++ b/base/file_util_posix.cc @@ -249,7 +249,7 @@ bool Delete(const FilePath& path, bool recursive) { return success; } -bool Move(const FilePath& from_path, const FilePath& to_path) { +bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) { base::ThreadRestrictions::AssertIOAllowed(); // Windows compatibility: if to_path exists, from_path and to_path // must be the same type, either both files, or both directories. @@ -1012,7 +1012,7 @@ FilePath GetHomeDir() { return FilePath("/tmp"); } -bool CopyFile(const FilePath& from_path, const FilePath& to_path) { +bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { base::ThreadRestrictions::AssertIOAllowed(); int infile = HANDLE_EINTR(open(from_path.value().c_str(), O_RDONLY)); if (infile < 0) diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc index d4ce43c..612f6cf 100644 --- a/base/file_util_unittest.cc +++ b/base/file_util_unittest.cc @@ -1149,8 +1149,8 @@ TEST_F(FileUtilTest, MoveNew) { ASSERT_TRUE(file_util::PathExists(dir_name_from)); // Create a file under the directory - FilePath file_name_from = - dir_name_from.Append(FILE_PATH_LITERAL("Move_Test_File.txt")); + FilePath txt_file_name(FILE_PATH_LITERAL("Move_Test_File.txt")); + FilePath file_name_from = dir_name_from.Append(txt_file_name); CreateTextFile(file_name_from, L"Gooooooooooooooooooooogle"); ASSERT_TRUE(file_util::PathExists(file_name_from)); @@ -1169,6 +1169,17 @@ TEST_F(FileUtilTest, MoveNew) { EXPECT_FALSE(file_util::PathExists(file_name_from)); EXPECT_TRUE(file_util::PathExists(dir_name_to)); EXPECT_TRUE(file_util::PathExists(file_name_to)); + + // Test path traversal. + file_name_from = dir_name_to.Append(txt_file_name); + file_name_to = dir_name_to.Append(FILE_PATH_LITERAL("..")); + file_name_to = file_name_to.Append(txt_file_name); + EXPECT_FALSE(file_util::Move(file_name_from, file_name_to)); + EXPECT_TRUE(file_util::PathExists(file_name_from)); + EXPECT_FALSE(file_util::PathExists(file_name_to)); + EXPECT_TRUE(file_util::MoveUnsafe(file_name_from, file_name_to)); + EXPECT_FALSE(file_util::PathExists(file_name_from)); + EXPECT_TRUE(file_util::PathExists(file_name_to)); } TEST_F(FileUtilTest, MoveExist) { @@ -1525,7 +1536,8 @@ TEST_F(FileUtilTest, CopyFile) { FilePath dest_file2(dir_name_from); dest_file2 = dest_file2.AppendASCII(".."); dest_file2 = dest_file2.AppendASCII("DestFile.txt"); - ASSERT_TRUE(file_util::CopyFile(file_name_from, dest_file2)); + ASSERT_FALSE(file_util::CopyFile(file_name_from, dest_file2)); + ASSERT_TRUE(file_util::CopyFileUnsafe(file_name_from, dest_file2)); FilePath dest_file2_test(dir_name_from); dest_file2_test = dest_file2_test.DirName(); diff --git a/base/file_util_win.cc b/base/file_util_win.cc index 537981e..5f6005a 100644 --- a/base/file_util_win.cc +++ b/base/file_util_win.cc @@ -138,7 +138,7 @@ bool DeleteAfterReboot(const FilePath& path) { MOVEFILE_REPLACE_EXISTING) != FALSE; } -bool Move(const FilePath& from_path, const FilePath& to_path) { +bool MoveUnsafe(const FilePath& from_path, const FilePath& to_path) { base::ThreadRestrictions::AssertIOAllowed(); // NOTE: I suspect we could support longer paths, but that would involve @@ -189,7 +189,7 @@ bool ReplaceFile(const FilePath& from_path, const FilePath& to_path) { return false; } -bool CopyFile(const FilePath& from_path, const FilePath& to_path) { +bool CopyFileUnsafe(const FilePath& from_path, const FilePath& to_path) { base::ThreadRestrictions::AssertIOAllowed(); // NOTE: I suspect we could support longer paths, but that would involve |