diff --git a/base/file_path.cc b/base/file_path.cc
index 094c699587d36..70cfb51823b3e 100644
--- a/base/file_path.cc
+++ b/base/file_path.cc
@@ -47,6 +47,8 @@ namespace {
const char* kCommonDoubleExtensionSuffixes[] = { "gz", "z", "bz2" };
const char* kCommonDoubleExtensions[] = { "user.js" };
+const FilePath::CharType kStringTerminator = FILE_PATH_LITERAL('\0');
+
// If this FilePath contains a drive letter specification, returns the
// position of the last character of the drive letter specification,
// otherwise returns npos. This can only be true on Windows, when a pathname
@@ -182,6 +184,9 @@ FilePath::FilePath(const FilePath& that) : path_(that.path_) {
}
FilePath::FilePath(const StringType& path) : path_(path) {
+ StringType::size_type nul_pos = path_.find(kStringTerminator);
+ if (nul_pos != StringType::npos)
+ path_.erase(nul_pos, StringType::npos);
}
FilePath::~FilePath() {
@@ -454,7 +459,17 @@ bool FilePath::MatchesExtension(const StringType& extension) const {
}
FilePath FilePath::Append(const StringType& component) const {
- DCHECK(!IsPathAbsolute(component));
+ const StringType* appended = &component;
+ StringType without_nuls;
+
+ StringType::size_type nul_pos = component.find(kStringTerminator);
+ if (nul_pos != StringType::npos) {
+ without_nuls = component.substr(0, nul_pos);
+ appended = &without_nuls;
+ }
+
+ DCHECK(!IsPathAbsolute(*appended));
+
if (path_.compare(kCurrentDirectory) == 0) {
// Append normally doesn't do any normalization, but as a special case,
// when appending to kCurrentDirectory, just return a new path for the
@@ -463,7 +478,7 @@ FilePath FilePath::Append(const StringType& component) const {
// it's likely in practice to wind up with FilePath objects containing
// only kCurrentDirectory when calling DirName on a single relative path
// component.
- return FilePath(component);
+ return FilePath(*appended);
}
FilePath new_path(path_);
@@ -472,7 +487,7 @@ FilePath FilePath::Append(const StringType& component) const {
// Don't append a separator if the path is empty (indicating the current
// directory) or if the path component is empty (indicating nothing to
// append).
- if (component.length() > 0 && new_path.path_.length() > 0) {
+ if (appended->length() > 0 && new_path.path_.length() > 0) {
// Don't append a separator if the path still ends with a trailing
// separator after stripping (indicating the root directory).
if (!IsSeparator(new_path.path_[new_path.path_.length() - 1])) {
@@ -483,7 +498,7 @@ FilePath FilePath::Append(const StringType& component) const {
}
}
- new_path.path_.append(component);
+ new_path.path_.append(*appended);
return new_path;
}
@@ -589,7 +604,7 @@ FilePath FilePath::FromUTF8Unsafe(const std::string& utf8) {
}
#endif
-void FilePath::WriteToPickle(Pickle* pickle) {
+void FilePath::WriteToPickle(Pickle* pickle) const {
#if defined(OS_WIN)
pickle->WriteString16(path_);
#else
@@ -606,6 +621,9 @@ bool FilePath::ReadFromPickle(PickleIterator* iter) {
return false;
#endif
+ if (path_.find(kStringTerminator) != StringType::npos)
+ return false;
+
return true;
}
diff --git a/base/file_path.h b/base/file_path.h
index 09e82693bb24e..81de702531de8 100644
--- a/base/file_path.h
+++ b/base/file_path.h
@@ -53,6 +53,8 @@
// between char[]-based pathnames on POSIX systems and wchar_t[]-based
// pathnames on Windows.
//
+// Paths can't contain NULs as a precaution agaist premature truncation.
+//
// Because a FilePath object should not be instantiated at the global scope,
// instead, use a FilePath::CharType[] and initialize it with
// FILE_PATH_LITERAL. At runtime, a FilePath object can be created from the
@@ -343,7 +345,7 @@ class BASE_EXPORT FilePath {
// AsUTF8Unsafe() for details.
static FilePath FromUTF8Unsafe(const std::string& utf8);
- void WriteToPickle(Pickle* pickle);
+ void WriteToPickle(Pickle* pickle) const;
bool ReadFromPickle(PickleIterator* iter);
// Normalize all path separators to backslash on Windows
diff --git a/base/file_path_unittest.cc b/base/file_path_unittest.cc
index ab25543652163..5a9e5cd6c8246 100644
--- a/base/file_path_unittest.cc
+++ b/base/file_path_unittest.cc
@@ -12,6 +12,9 @@
// This macro helps avoid wrapped lines in the test structs.
#define FPL(x) FILE_PATH_LITERAL(x)
+// This macro constructs strings which can contain NULs.
+#define FPS(x) FilePath::StringType(FPL(x), arraysize(FPL(x)) - 1)
+
struct UnaryTestData {
const FilePath::CharType* input;
const FilePath::CharType* expected;
@@ -1115,6 +1118,40 @@ TEST_F(FilePathTest, FromUTF8Unsafe_And_AsUTF8Unsafe) {
}
}
+TEST_F(FilePathTest, ConstructWithNUL) {
+ // Assert FPS() works.
+ ASSERT_TRUE(FPS("a\0b").length() == 3);
+
+ // Test constructor strips '\0'
+ FilePath path(FPS("a\0b"));
+ EXPECT_TRUE(path.value().length() == 1);
+ EXPECT_EQ(path.value(), FPL("a"));
+}
+
+TEST_F(FilePathTest, AppendWithNUL) {
+ // Assert FPS() works.
+ ASSERT_TRUE(FPS("b\0b").length() == 3);
+
+ // Test Append() strips '\0'
+ FilePath path(FPL("a"));
+ path = path.Append(FPS("b\0b"));
+ EXPECT_TRUE(path.value().length() == 3);
+#if defined(FILE_PATH_USES_WIN_SEPARATORS)
+ EXPECT_EQ(path.value(), FPL("a\\b"));
+#else
+ EXPECT_EQ(path.value(), FPL("a/b"));
+#endif
+}
+
+TEST_F(FilePathTest, ReferencesParentWithNUL) {
+ // Assert FPS() works.
+ ASSERT_TRUE(FPS("..\0").length() == 3);
+
+ // Test ReferencesParent() doesn't break with "..\0"
+ FilePath path(FPS("..\0"));
+ EXPECT_TRUE(path.ReferencesParent());
+}
+
#if defined(FILE_PATH_USES_WIN_SEPARATORS)
TEST_F(FilePathTest, NormalizePathSeparators) {
const struct UnaryTestData cases[] = {
diff --git a/content/browser/download/save_package_unittest.cc b/content/browser/download/save_package_unittest.cc
index 89fb89a9f7df5..a34e138c1c94c 100644
--- a/content/browser/download/save_package_unittest.cc
+++ b/content/browser/download/save_package_unittest.cc
@@ -109,7 +109,7 @@ class SavePackageTest : public RenderViewHostImplTestHarness {
temp_dir_.path().AppendASCII("testfile_files"));
// We need to construct a path that is *almost* kMaxFilePathLength long
- long_file_name.resize(kMaxFilePathLength + long_file_name.length());
+ long_file_name.reserve(kMaxFilePathLength + long_file_name.length());
while (long_file_name.length() < kMaxFilePathLength)
long_file_name += long_file_name;
long_file_name.resize(
diff --git a/ipc/ipc_message_utils.cc b/ipc/ipc_message_utils.cc
index dd310c1a3e5ac..432853c6dfb6b 100644
--- a/ipc/ipc_message_utils.cc
+++ b/ipc/ipc_message_utils.cc
@@ -489,20 +489,13 @@ void ParamTraits<base::FileDescriptor>::Log(const param_type& p,
#endif // defined(OS_POSIX)
void ParamTraits<FilePath>::Write(Message* m, const param_type& p) {
- ParamTraits<FilePath::StringType>::Write(m, p.value());
+ p.WriteToPickle(m);
}
bool ParamTraits<FilePath>::Read(const Message* m,
PickleIterator* iter,
param_type* r) {
- FilePath::StringType value;
- if (!ParamTraits<FilePath::StringType>::Read(m, iter, &value))
- return false;
- // Reject embedded NULs as they can cause security checks to go awry.
- if (value.find(FILE_PATH_LITERAL('\0')) != FilePath::StringType::npos)
- return false;
- *r = FilePath(value);
- return true;
+ return r->ReadFromPickle(iter);
}
void ParamTraits<FilePath>::Log(const param_type& p, std::string* l) {
diff --git a/ipc/ipc_message_utils_unittest.cc b/ipc/ipc_message_utils_unittest.cc
index 2a49b9a56980d..f9cb553a1f090 100644
--- a/ipc/ipc_message_utils_unittest.cc
+++ b/ipc/ipc_message_utils_unittest.cc
@@ -55,17 +55,19 @@ TEST(IPCMessageUtilsTest, NestedMessages) {
// Tests that detection of various bad parameters is working correctly.
TEST(IPCMessageUtilsTest, ParameterValidation) {
+ FilePath::StringType ok_string(FILE_PATH_LITERAL("hello"), 5);
+ FilePath::StringType bad_string(FILE_PATH_LITERAL("hel\0o"), 5);
+
+ // Change this if ParamTraits<FilePath>::Write() changes.
IPC::Message message;
- FilePath::StringType okString(FILE_PATH_LITERAL("hello"), 5);
- FilePath::StringType badString(FILE_PATH_LITERAL("hel\0o"), 5);
- FilePath okPath(okString);
- FilePath badPath(badString);
- ParamTraits<FilePath>::Write(&message, okPath);
- ParamTraits<FilePath>::Write(&message, badPath);
+ ParamTraits<FilePath::StringType>::Write(&message, ok_string);
+ ParamTraits<FilePath::StringType>::Write(&message, bad_string);
PickleIterator iter(message);
- ASSERT_TRUE(ParamTraits<FilePath>::Read(&message, &iter, &okPath));
- ASSERT_FALSE(ParamTraits<FilePath>::Read(&message, &iter, &badPath));
+ FilePath ok_path;
+ FilePath bad_path;
+ ASSERT_TRUE(ParamTraits<FilePath>::Read(&message, &iter, &ok_path));
+ ASSERT_FALSE(ParamTraits<FilePath>::Read(&message, &iter, &bad_path));
}
} // namespace