summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--base/file_util.cc12
-rw-r--r--base/file_util.h12
-rw-r--r--base/file_util_mac.mm2
-rw-r--r--base/file_util_posix.cc4
-rw-r--r--base/file_util_unittest.cc18
-rw-r--r--base/file_util_win.cc4
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