summaryrefslogtreecommitdiffstats
path: root/net/ftp
diff options
context:
space:
mode:
Diffstat (limited to 'net/ftp')
-rw-r--r--net/ftp/ftp_ctrl_response_buffer.cc2
-rw-r--r--net/ftp/ftp_directory_listing_buffer_unittest.cc1
-rw-r--r--net/ftp/ftp_directory_listing_parser_ls.cc24
-rw-r--r--net/ftp/ftp_directory_listing_parser_ls_unittest.cc28
-rw-r--r--net/ftp/ftp_directory_listing_parser_mlsd.cc18
-rw-r--r--net/ftp/ftp_network_transaction.cc11
-rw-r--r--net/ftp/ftp_network_transaction_unittest.cc57
-rw-r--r--net/ftp/ftp_util.cc16
8 files changed, 137 insertions, 20 deletions
diff --git a/net/ftp/ftp_ctrl_response_buffer.cc b/net/ftp/ftp_ctrl_response_buffer.cc
index 950e1e0..4aeef1f 100644
--- a/net/ftp/ftp_ctrl_response_buffer.cc
+++ b/net/ftp/ftp_ctrl_response_buffer.cc
@@ -91,7 +91,7 @@ FtpCtrlResponseBuffer::ParsedLine FtpCtrlResponseBuffer::ParseLine(
ParsedLine result;
if (line.length() >= 3) {
- if (base::StringToInt(line.substr(0, 3), &result.status_code))
+ if (base::StringToInt(line.begin(), line.begin() + 3, &result.status_code))
result.has_status_code = (100 <= result.status_code &&
result.status_code <= 599);
if (result.has_status_code && line.length() >= 4 && line[3] == ' ') {
diff --git a/net/ftp/ftp_directory_listing_buffer_unittest.cc b/net/ftp/ftp_directory_listing_buffer_unittest.cc
index 86fc6e7..683e2f7 100644
--- a/net/ftp/ftp_directory_listing_buffer_unittest.cc
+++ b/net/ftp/ftp_directory_listing_buffer_unittest.cc
@@ -41,6 +41,7 @@ TEST(FtpDirectoryListingBufferTest, Parse) {
"dir-listing-ls-16",
"dir-listing-ls-17",
"dir-listing-ls-18",
+ "dir-listing-ls-19",
"dir-listing-mlsd-1",
"dir-listing-mlsd-2",
"dir-listing-netware-1",
diff --git a/net/ftp/ftp_directory_listing_parser_ls.cc b/net/ftp/ftp_directory_listing_parser_ls.cc
index 89fc3d8..40546c8 100644
--- a/net/ftp/ftp_directory_listing_parser_ls.cc
+++ b/net/ftp/ftp_directory_listing_parser_ls.cc
@@ -126,8 +126,20 @@ bool FtpDirectoryListingParserLs::ConsumeLine(const string16& line) {
// We may receive file names containing spaces, which can make the number of
// columns arbitrarily large. We will handle that later. For now just make
- // sure we have all the columns that should normally be there.
- if (columns.size() < 7U + column_offset)
+ // sure we have all the columns that should normally be there:
+ //
+ // 1. permission listing
+ // 2. number of links (optional)
+ // 3. owner name
+ // 4. group name (optional)
+ // 5. size in bytes
+ // 6. month
+ // 7. day of month
+ // 8. year or time
+ //
+ // The number of optional columns is stored in |column_offset|
+ // and is between 0 and 2 (inclusive).
+ if (columns.size() < 6U + column_offset)
return false;
if (!LooksLikeUnixPermissionsListing(columns[0]))
@@ -165,6 +177,14 @@ bool FtpDirectoryListingParserLs::ConsumeLine(const string16& line) {
}
entry.name = FtpUtil::GetStringPartAfterColumns(line, 6 + column_offset);
+
+ if (entry.name.empty()) {
+ // Some FTP servers send listing entries with empty names. It's not obvious
+ // how to display such an entry, so we ignore them. We don't want to make
+ // the parsing fail at this point though. Other entries can still be useful.
+ return true;
+ }
+
if (entry.type == FtpDirectoryListingEntry::SYMLINK) {
string16::size_type pos = entry.name.rfind(ASCIIToUTF16(" -> "));
diff --git a/net/ftp/ftp_directory_listing_parser_ls_unittest.cc b/net/ftp/ftp_directory_listing_parser_ls_unittest.cc
index 4b36544..3e777cc 100644
--- a/net/ftp/ftp_directory_listing_parser_ls_unittest.cc
+++ b/net/ftp/ftp_directory_listing_parser_ls_unittest.cc
@@ -91,6 +91,28 @@ TEST_F(FtpDirectoryListingParserLsTest, Good) {
}
}
+TEST_F(FtpDirectoryListingParserLsTest, Ignored) {
+ const char* ignored_cases[] = {
+ "drwxr-xr-x 2 0 0 4096 Mar 18 2007 ", // http://crbug.com/60065
+
+ // Tests important for security: verify that after we detect the column
+ // offset we don't try to access invalid memory on malformed input.
+ "drwxr-xr-x 3 ftp ftp 4096 May 15 18:11",
+ "drwxr-xr-x 3 ftp 4096 May 15 18:11",
+ "drwxr-xr-x folder 0 May 15 18:11",
+ };
+ for (size_t i = 0; i < arraysize(ignored_cases); i++) {
+ SCOPED_TRACE(base::StringPrintf("Test[%" PRIuS "]: %s", i,
+ ignored_cases[i]));
+
+ net::FtpDirectoryListingParserLs parser(GetMockCurrentTime());
+ EXPECT_TRUE(parser.ConsumeLine(UTF8ToUTF16(ignored_cases[i])));
+ EXPECT_FALSE(parser.EntryAvailable());
+ EXPECT_TRUE(parser.OnEndOfInput());
+ EXPECT_FALSE(parser.EntryAvailable());
+ }
+}
+
TEST_F(FtpDirectoryListingParserLsTest, Bad) {
const char* bad_cases[] = {
" foo",
@@ -112,12 +134,6 @@ TEST_F(FtpDirectoryListingParserLsTest, Bad) {
"d-wx-wx-wt++ 4 ftp 989 512 Dec 8 15:54 incoming",
"d-wx-wx-wt$ 4 ftp 989 512 Dec 8 15:54 incoming",
"-qqqqqqqqq+ 2 sys 512 Mar 27 2009 pub",
-
- // Tests important for security: verify that after we detect the column
- // offset we don't try to access invalid memory on malformed input.
- "drwxr-xr-x 3 ftp ftp 4096 May 15 18:11",
- "drwxr-xr-x 3 ftp 4096 May 15 18:11",
- "drwxr-xr-x folder 0 May 15 18:11",
};
for (size_t i = 0; i < arraysize(bad_cases); i++) {
net::FtpDirectoryListingParserLs parser(GetMockCurrentTime());
diff --git a/net/ftp/ftp_directory_listing_parser_mlsd.cc b/net/ftp/ftp_directory_listing_parser_mlsd.cc
index fa9a44e..d8ae618 100644
--- a/net/ftp/ftp_directory_listing_parser_mlsd.cc
+++ b/net/ftp/ftp_directory_listing_parser_mlsd.cc
@@ -28,15 +28,23 @@ bool MlsdDateListingToTime(const string16& text, base::Time* time) {
if (text.length() < 14)
return false;
- if (!base::StringToInt(text.substr(0, 4), &time_exploded.year))
+ if (!base::StringToInt(text.begin(), text.begin() + 4, &time_exploded.year))
return false;
- if (!base::StringToInt(text.substr(4, 2), &time_exploded.month))
+ if (!base::StringToInt(text.begin() + 4,
+ text.begin() + 6,
+ &time_exploded.month))
return false;
- if (!base::StringToInt(text.substr(6, 2), &time_exploded.day_of_month))
+ if (!base::StringToInt(text.begin() + 6,
+ text.begin() + 8,
+ &time_exploded.day_of_month))
return false;
- if (!base::StringToInt(text.substr(8, 2), &time_exploded.hour))
+ if (!base::StringToInt(text.begin() + 8,
+ text.begin() + 10,
+ &time_exploded.hour))
return false;
- if (!base::StringToInt(text.substr(10, 2), &time_exploded.minute))
+ if (!base::StringToInt(text.begin() + 10,
+ text.begin() + 12,
+ &time_exploded.minute))
return false;
// We don't know the time zone of the server, so just use local time.
diff --git a/net/ftp/ftp_network_transaction.cc b/net/ftp/ftp_network_transaction.cc
index 02e4be0..bc1c2a9 100644
--- a/net/ftp/ftp_network_transaction.cc
+++ b/net/ftp/ftp_network_transaction.cc
@@ -977,8 +977,14 @@ int FtpNetworkTransaction::ProcessResponseSIZE(
return Stop(ERR_INVALID_RESPONSE);
if (size < 0)
return Stop(ERR_INVALID_RESPONSE);
- response_.expected_content_size = size;
- resource_type_ = RESOURCE_TYPE_FILE;
+
+ // Some FTP servers respond with success to the SIZE command
+ // for directories, and return 0 size. Make sure we don't set
+ // the resource type to file if that's the case.
+ if (size > 0) {
+ response_.expected_content_size = size;
+ resource_type_ = RESOURCE_TYPE_FILE;
+ }
break;
case ERROR_CLASS_INFO_NEEDED:
break;
@@ -1298,6 +1304,7 @@ void FtpNetworkTransaction::RecordDataConnectionError(int result) {
type = NET_ERROR_OK;
break;
case ERR_ACCESS_DENIED:
+ case ERR_NETWORK_ACCESS_DENIED:
type = NET_ERROR_ACCESS_DENIED;
break;
case ERR_TIMED_OUT:
diff --git a/net/ftp/ftp_network_transaction_unittest.cc b/net/ftp/ftp_network_transaction_unittest.cc
index b504ae8..9c9f62e 100644
--- a/net/ftp/ftp_network_transaction_unittest.cc
+++ b/net/ftp/ftp_network_transaction_unittest.cc
@@ -231,6 +231,27 @@ class FtpSocketDataProviderDirectoryListingWithPasvFallback
FtpSocketDataProviderDirectoryListingWithPasvFallback);
};
+class FtpSocketDataProviderDirectoryListingZeroSize
+ : public FtpSocketDataProviderDirectoryListing {
+ public:
+ FtpSocketDataProviderDirectoryListingZeroSize() {
+ }
+
+ virtual MockWriteResult OnWrite(const std::string& data) {
+ if (InjectFault())
+ return MockWriteResult(true, data.length());
+ switch (state()) {
+ case PRE_SIZE:
+ return Verify("SIZE /\r\n", data, PRE_CWD, "213 0\r\n");
+ default:
+ return FtpSocketDataProviderDirectoryListing::OnWrite(data);
+ }
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderDirectoryListingZeroSize);
+};
+
class FtpSocketDataProviderVMSDirectoryListing : public FtpSocketDataProvider {
public:
FtpSocketDataProviderVMSDirectoryListing() {
@@ -385,6 +406,31 @@ class FtpSocketDataProviderFileDownloadWithPasvFallback
DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderFileDownloadWithPasvFallback);
};
+class FtpSocketDataProviderFileDownloadZeroSize
+ : public FtpSocketDataProviderFileDownload {
+ public:
+ FtpSocketDataProviderFileDownloadZeroSize() {
+ }
+
+ virtual MockWriteResult OnWrite(const std::string& data) {
+ if (InjectFault())
+ return MockWriteResult(true, data.length());
+ switch (state()) {
+ case PRE_SIZE:
+ return Verify("SIZE /file\r\n", data, PRE_CWD,
+ "213 0\r\n");
+ case PRE_CWD:
+ return Verify("CWD /file\r\n", data, PRE_RETR,
+ "550 not a directory\r\n");
+ default:
+ return FtpSocketDataProviderFileDownload::OnWrite(data);
+ }
+ }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(FtpSocketDataProviderFileDownloadZeroSize);
+};
+
class FtpSocketDataProviderVMSFileDownload : public FtpSocketDataProvider {
public:
FtpSocketDataProviderVMSFileDownload() {
@@ -807,6 +853,12 @@ TEST_F(FtpNetworkTransactionTest, DirectoryTransactionMultilineWelcomeShort) {
ExecuteTransaction(&ctrl_socket, "ftp://host", OK);
}
+// Regression test for http://crbug.com/60555.
+TEST_F(FtpNetworkTransactionTest, DirectoryTransactionZeroSize) {
+ FtpSocketDataProviderDirectoryListingZeroSize ctrl_socket;
+ ExecuteTransaction(&ctrl_socket, "ftp://host", OK);
+}
+
TEST_F(FtpNetworkTransactionTest, DirectoryTransactionVMS) {
FtpSocketDataProviderVMSDirectoryListing ctrl_socket;
ExecuteTransaction(&ctrl_socket, "ftp://host/dir", OK);
@@ -873,6 +925,11 @@ TEST_F(FtpNetworkTransactionTest, DownloadTransactionShortReads5) {
ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK);
}
+TEST_F(FtpNetworkTransactionTest, DownloadTransactionZeroSize) {
+ FtpSocketDataProviderFileDownloadZeroSize ctrl_socket;
+ ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK);
+}
+
TEST_F(FtpNetworkTransactionTest, DownloadTransactionVMS) {
FtpSocketDataProviderVMSFileDownload ctrl_socket;
ExecuteTransaction(&ctrl_socket, "ftp://host/file", OK);
diff --git a/net/ftp/ftp_util.cc b/net/ftp/ftp_util.cc
index aeb2355..2633e9d 100644
--- a/net/ftp/ftp_util.cc
+++ b/net/ftp/ftp_util.cc
@@ -162,17 +162,25 @@ bool FtpUtil::LsDateListingToTime(const string16& month, const string16& day,
if (!base::StringToInt(rest, &time_exploded.year)) {
// Maybe it's time. Does it look like time (HH:MM)?
if (rest.length() == 5 && rest[2] == ':') {
- if (!base::StringToInt(rest.substr(0, 2), &time_exploded.hour))
+ if (!base::StringToInt(rest.begin(),
+ rest.begin() + 2,
+ &time_exploded.hour))
return false;
- if (!base::StringToInt(rest.substr(3, 2), &time_exploded.minute))
+ if (!base::StringToInt(rest.begin() + 3,
+ rest.begin() + 5,
+ &time_exploded.minute))
return false;
} else if (rest.length() == 4 && rest[1] == ':') {
// Sometimes it's just H:MM.
- if (!base::StringToInt(rest.substr(0, 1), &time_exploded.hour))
+ if (!base::StringToInt(rest.begin(),
+ rest.begin() + 1,
+ &time_exploded.hour))
return false;
- if (!base::StringToInt(rest.substr(2, 2), &time_exploded.minute))
+ if (!base::StringToInt(rest.begin() + 2,
+ rest.begin() + 4,
+ &time_exploded.minute))
return false;
} else {
return false;
generated by cgit v1.1 at 2025-11-06 23:53:37 +0000