Fix yet another silly DCHECK in the FTP code.

net/ftp/ftp_network_transaction.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 "net/ftp/ftp_network_transaction.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 18 matching lines @@
 #include "url/url_constants.h"
 
 namespace net {
 
 namespace {
 
 const char kCRLF[] = "\r\n";
 
 const int kCtrlBufLen = 1024;
 
-// Returns true if |input| can be safely used as a part of FTP command.
-bool IsValidFTPCommandString(const std::string& input) {
+// Returns true if |input| can be safely used as a part of an FTP command.
+bool IsValidFTPCommandSubstring(const std::string& input) {
   // RFC 959 only allows ASCII strings, but at least Firefox can send non-ASCII
   // characters in the command if the request path contains them. To be
   // compatible, we do the same and allow non-ASCII characters in a command.
 
   // Protect agains newline injection attack.
   if (input.find_first_of("\r\n") != std::string::npos)
     return false;
 
   return true;
 }
@@ skipping 404 matching lines @@
                                           const std::string& command_for_log,
                                           Command cmd) {
   // If we send a new command when we still have unprocessed responses
   // for previous commands, the response receiving code will have no way to know
   // which responses are for which command.
   DCHECK(!ctrl_response_buffer_->ResponseAvailable());
 
   DCHECK(!write_command_buf_.get());
   DCHECK(!write_buf_.get());
 
-  if (!IsValidFTPCommandString(command)) {
+  if (!IsValidFTPCommandSubstring(command)) {
     // Callers should validate the command themselves and return a more specific
     // error code.
     NOTREACHED();
     return Stop(ERR_UNEXPECTED);
   }
 
   command_sent_ = cmd;
 
   write_command_buf_ = new IOBufferWithSize(command.length() + 2);
   write_buf_ = new DrainableIOBuffer(write_command_buf_.get(),
@@ skipping 22 matching lines @@
     path.append(gurl_path);
   }
   // Make sure that if the path is expected to be a file, it won't end
   // with a trailing slash.
   if (!is_directory && path.length() > 1 && path.back() == '/')
     path.erase(path.length() - 1);
   UnescapeRule::Type unescape_rules =
       UnescapeRule::SPACES |
       UnescapeRule::URL_SPECIAL_CHARS_EXCEPT_PATH_SEPARATORS;
   // This may unescape to non-ASCII characters, but we allow that. See the
-  // comment for IsValidFTPCommandString.
+  // comment for IsValidFTPCommandSubstring.
   path = UnescapeURLComponent(path, unescape_rules);
 
   if (system_type_ == SYSTEM_TYPE_VMS) {
     if (is_directory)
       path = FtpUtil::UnixDirectoryPathToVMS(path);
     else
       path = FtpUtil::UnixFilePathToVMS(path);
   }
 
-  DCHECK(IsValidFTPCommandString(path));
+  DCHECK(IsValidFTPCommandSubstring(path));
   return path;
 }
 
 void FtpNetworkTransaction::DetectTypecode() {
   if (!request_->url.has_path())
     return;
   std::string gurl_path(request_->url.path());
 
   // Extract the typecode, see RFC 1738 section 3.2.2. FTP url-path.
   std::string::size_type pos = gurl_path.rfind(';');
@@ skipping 220 matching lines @@
   }
   return OK;
 }
 
 // FTP Commands and responses
 
 // USER Command.
 int FtpNetworkTransaction::DoCtrlWriteUSER() {
   std::string command = "USER " + base::UTF16ToUTF8(credentials_.username());
 
-  if (!IsValidFTPCommandString(command))
+  if (!IsValidFTPCommandSubstring(command))
     return Stop(ERR_MALFORMED_IDENTITY);
 
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, "USER ***", COMMAND_USER);
 }
 
 int FtpNetworkTransaction::ProcessResponseUSER(
     const FtpCtrlResponse& response) {
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
       return Stop(ERR_INVALID_RESPONSE);
     case ERROR_CLASS_OK:
       next_state_ = STATE_CTRL_WRITE_SYST;
       break;
     case ERROR_CLASS_INFO_NEEDED:
       next_state_ = STATE_CTRL_WRITE_PASS;
       break;
     case ERROR_CLASS_TRANSIENT_ERROR:
     case ERROR_CLASS_PERMANENT_ERROR:
       response_.needs_auth = true;
       return Stop(GetNetErrorCodeForFtpResponseCode(response.status_code));
   }
   return OK;
 }
 
 // PASS command.
 int FtpNetworkTransaction::DoCtrlWritePASS() {
   std::string command = "PASS " + base::UTF16ToUTF8(credentials_.password());
 
-  if (!IsValidFTPCommandString(command))
+  if (!IsValidFTPCommandSubstring(command))
     return Stop(ERR_MALFORMED_IDENTITY);
 
   next_state_ = STATE_CTRL_READ;
   return SendFtpCommand(command, "PASS ***", COMMAND_PASS);
 }
 
 int FtpNetworkTransaction::ProcessResponsePASS(
     const FtpCtrlResponse& response) {
   switch (GetErrorClass(response.status_code)) {
     case ERROR_CLASS_INITIATED:
@@ skipping 89 matching lines @@
         line = line.substr(quote_pos + 1);
         quote_pos = line.find('"');
         if (quote_pos == std::string::npos)
           return Stop(ERR_INVALID_RESPONSE);
         line = line.substr(0, quote_pos);
       }
       if (system_type_ == SYSTEM_TYPE_VMS)
         line = FtpUtil::VMSPathToUnix(line);
       if (!line.empty() && line.back() == '/')
         line.erase(line.length() - 1);
+      // Fail if the "path" contains characters not allowed in commands.
+      // This does mean that files with CRs or LFs in their names aren't
+      // handled, but that's probably for the best.
+      if (!IsValidFTPCommandSubstring(line))
+        return Stop(ERR_INVALID_RESPONSE);
       current_remote_directory_ = line;
       next_state_ = STATE_CTRL_WRITE_TYPE;
       break;
     }
     case ERROR_CLASS_INFO_NEEDED:
       return Stop(ERR_INVALID_RESPONSE);
     case ERROR_CLASS_TRANSIENT_ERROR:
       return Stop(GetNetErrorCodeForFtpResponseCode(response.status_code));
     case ERROR_CLASS_PERMANENT_ERROR:
       return Stop(GetNetErrorCodeForFtpResponseCode(response.status_code));
@@ skipping 447 matching lines @@
   if (!had_error_type[type]) {
     had_error_type[type] = true;
     UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorHappened",
         type, NUM_OF_NET_ERROR_TYPES);
   }
   UMA_HISTOGRAM_ENUMERATION("Net.FtpDataConnectionErrorCount",
       type, NUM_OF_NET_ERROR_TYPES);
 }
 
 }  // namespace net