Don't allow '\0' characters in FilePath.

base/file_path.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/file_path.h"
 
 #include <string.h>
 #include <algorithm>
 
 #include "base/basictypes.h"
@@ skipping 29 matching lines @@
 
 const FilePath::CharType FilePath::kExtensionSeparator = FILE_PATH_LITERAL('.');
 
 typedef FilePath::StringType StringType;
 
 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
 // begins with a letter followed by a colon.  On other platforms, this always
 // returns npos.
 StringType::size_type FindDriveLetter(const StringType& path) {
 #if defined(FILE_PATH_USES_DRIVE_LETTERS)
   // This is dependent on an ASCII-based character set, but that's a
   // reasonable assumption.  iswalpha can be too inclusive here.
   if (path.length() >= 2 && path[1] == L':' &&
@@ skipping 115 matching lines @@
 
 }  // namespace
 
 FilePath::FilePath() {
 }
 
 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);

[Chris Evans, 2013/01/08 09:41:01]

I'm ok either way, but just a thought to throw out there: what about we
CHECK(path_.find(kS..) == StringType::npos) instead? Both here and in append.
It's simpler code, and also a simpler contract. I worry that calling code might
have precomputed a length expectation and then get surprised when we truncated
the array that was passed in.

Maybe the benefits outweight the drawbacks, maybe not. What do you and Tom
think?

 }
 
 FilePath::~FilePath() {
 }
 
 FilePath& FilePath::operator=(const FilePath& that) {
   path_ = that.path_;
   return *this;
 }
 
@@ skipping 252 matching lines @@
 
   StringType current_extension = Extension();
 
   if (current_extension.length() != extension.length())
     return false;
 
   return FilePath::CompareEqualIgnoreCase(extension, current_extension);
 }
 
 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
     // component argument.  Appending component to kCurrentDirectory would
     // serve no purpose other than needlessly lengthening the path, and
     // 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_);
   new_path.StripTrailingSeparatorsInternal();
 
   // 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])) {
       // Don't append a separator if the path is just a drive letter.
       if (FindDriveLetter(new_path.path_) + 1 != new_path.path_.length()) {
         new_path.path_.append(1, kSeparators[0]);
       }
     }
   }
 
-  new_path.path_.append(component);
+  new_path.path_.append(*appended);
   return new_path;
 }
 
 FilePath FilePath::Append(const FilePath& component) const {
   return Append(component.value());
 }
 
 FilePath FilePath::AppendASCII(const base::StringPiece& component) const {
   DCHECK(IsStringASCII(component));
 #if defined(OS_WIN)
@@ skipping 102 matching lines @@
 
 bool FilePath::ReadFromPickle(PickleIterator* iter) {
 #if defined(OS_WIN)
   if (!iter->ReadString16(&path_))
     return false;
 #else
   if (!iter->ReadString(&path_))
     return false;
 #endif
 
+  if (path_.find(kStringTerminator) != StringType::npos)
+    return false;
+
   return true;
 }
 
 #if defined(OS_WIN)
 // Windows specific implementation of file string comparisons
 
 int FilePath::CompareIgnoreCase(const StringType& string1,
                                 const StringType& string2) {
   // Perform character-wise upper case comparison rather than using the
   // fully Unicode-aware CompareString(). For details see:
@@ skipping 610 matching lines @@
   }
   return FilePath(copy);
 #else
   return *this;
 #endif
 }
 
 void PrintTo(const FilePath& path, std::ostream* out) {
   *out << path.value();
 }