diff options
-rw-r--r-- | base/file_path.cc | 28 | ||||
-rw-r--r-- | base/file_path.h | 4 | ||||
-rw-r--r-- | base/file_path_unittest.cc | 37 | ||||
-rw-r--r-- | content/browser/download/save_package_unittest.cc | 2 | ||||
-rw-r--r-- | ipc/ipc_message_utils.cc | 11 | ||||
-rw-r--r-- | ipc/ipc_message_utils_unittest.cc | 18 |
6 files changed, 76 insertions, 24 deletions
diff --git a/base/file_path.cc b/base/file_path.cc index 094c699..70cfb51 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 09e8269..81de702 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 ab25543..5a9e5cd 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 89fb89a..a34e138 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 babecdc..1f79e23 100644 --- a/ipc/ipc_message_utils.cc +++ b/ipc/ipc_message_utils.cc @@ -499,20 +499,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 fe2c4e3..d51f532 100644 --- a/ipc/ipc_message_utils_unittest.cc +++ b/ipc/ipc_message_utils_unittest.cc @@ -53,17 +53,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 IPC |