Commit 089ec1a2 authored by mkolom's avatar mkolom Committed by Commit bot

Fix error handling in POSIX version of the base::File::GetLength.

According to the base/files/file.h, the GetLength() method should return
a negative number on failure.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.win:win10_chromium_x64_rel_ng

Review-Url: https://codereview.chromium.org/2404823002
Cr-Commit-Position: refs/heads/master@{#437512}
parent c216c500
...@@ -321,7 +321,7 @@ int64_t File::GetLength() { ...@@ -321,7 +321,7 @@ int64_t File::GetLength() {
stat_wrapper_t file_info; stat_wrapper_t file_info;
if (CallFstat(file_.get(), &file_info)) if (CallFstat(file_.get(), &file_info))
return false; return -1;
return file_info.st_size; return file_info.st_size;
} }
......
...@@ -31,7 +31,7 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -31,7 +31,7 @@ bool MemoryMappedFile::MapFileRegionToMemory(
if (region == MemoryMappedFile::Region::kWholeFile) { if (region == MemoryMappedFile::Region::kWholeFile) {
int64_t file_len = file_.GetLength(); int64_t file_len = file_.GetLength();
if (file_len == -1) { if (file_len < 0) {
DPLOG(ERROR) << "fstat " << file_.GetPlatformFile(); DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
return false; return false;
} }
...@@ -78,7 +78,12 @@ bool MemoryMappedFile::MapFileRegionToMemory( ...@@ -78,7 +78,12 @@ bool MemoryMappedFile::MapFileRegionToMemory(
// POSIX won't auto-extend the file when it is written so it must first // POSIX won't auto-extend the file when it is written so it must first
// be explicitly extended to the maximum size. Zeros will fill the new // be explicitly extended to the maximum size. Zeros will fill the new
// space. // space.
file_.SetLength(std::max(file_.GetLength(), region.offset + region.size)); auto file_len = file_.GetLength();
if (file_len < 0) {
DPLOG(ERROR) << "fstat " << file_.GetPlatformFile();
return false;
}
file_.SetLength(std::max(file_len, region.offset + region.size));
flags |= PROT_READ | PROT_WRITE; flags |= PROT_READ | PROT_WRITE;
break; break;
} }
......
...@@ -27,10 +27,23 @@ std::unique_ptr<char[]> ReadWavFile(const base::FilePath& wav_filename, ...@@ -27,10 +27,23 @@ std::unique_ptr<char[]> ReadWavFile(const base::FilePath& wav_filename,
return nullptr; return nullptr;
} }
size_t wav_file_length = wav_file.GetLength(); int64_t wav_file_length64 = wav_file.GetLength();
if (wav_file_length64 < 0) {
PLOG(ERROR) << "GetLength " << wav_file.GetPlatformFile();
return nullptr;
}
if (wav_file_length64 > std::numeric_limits<int>::max()) {
LOG(ERROR) << "File is too big: " << wav_filename.value();
return nullptr;
}
int wav_file_length = static_cast<int>(wav_file_length64);
if (!wav_file_length) {
LOG(ERROR) << "Input file is empty: " << wav_filename.value();
return nullptr;
}
std::unique_ptr<char[]> data(new char[wav_file_length]); std::unique_ptr<char[]> data(new char[wav_file_length]);
size_t read_bytes = wav_file.Read(0, data.get(), wav_file_length); int read_bytes = wav_file.Read(0, data.get(), wav_file_length);
if (read_bytes != wav_file_length) { if (read_bytes != wav_file_length) {
LOG(ERROR) << "Failed to read all bytes of " << wav_filename.value(); LOG(ERROR) << "Failed to read all bytes of " << wav_filename.value();
return nullptr; return nullptr;
......
...@@ -94,7 +94,7 @@ void ExpectLogosEqual(const EncodedLogo& expected_logo, ...@@ -94,7 +94,7 @@ void ExpectLogosEqual(const EncodedLogo& expected_logo,
void ShortenFile(base::FilePath path) { void ShortenFile(base::FilePath path) {
base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_WRITE); base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_WRITE);
int64_t file_length = file.GetLength(); int64_t file_length = file.GetLength();
ASSERT_NE(file_length, 0); ASSERT_GT(file_length, 0);
file.SetLength(file_length - 1); file.SetLength(file_length - 1);
} }
......
...@@ -349,6 +349,8 @@ bool RulesetService::IndexRuleset(base::File unindexed_ruleset_file, ...@@ -349,6 +349,8 @@ bool RulesetService::IndexRuleset(base::File unindexed_ruleset_file,
"SubresourceFilter.IndexRuleset.CPUDuration"); "SubresourceFilter.IndexRuleset.CPUDuration");
int64_t unindexed_ruleset_size = unindexed_ruleset_file.GetLength(); int64_t unindexed_ruleset_size = unindexed_ruleset_file.GetLength();
if (unindexed_ruleset_size < 0)
return false;
CopyingFileInputStream copying_stream(std::move(unindexed_ruleset_file)); CopyingFileInputStream copying_stream(std::move(unindexed_ruleset_file));
google::protobuf::io::CopyingInputStreamAdaptor zero_copy_stream_adaptor( google::protobuf::io::CopyingInputStreamAdaptor zero_copy_stream_adaptor(
&copying_stream, 4096 /* buffer_size */); &copying_stream, 4096 /* buffer_size */);
......
...@@ -83,6 +83,7 @@ class AudioInputDebugWriterTest ...@@ -83,6 +83,7 @@ class AudioInputDebugWriterTest
// 0 4 "RIFF" // 0 4 "RIFF"
EXPECT_EQ(0, strncmp(wav_header, "RIFF", 4)); EXPECT_EQ(0, strncmp(wav_header, "RIFF", 4));
// 4 4 <file length - 8> // 4 4 <file length - 8>
ASSERT_GT(file_length, 8);
EXPECT_EQ(static_cast<uint64_t>(file_length - 8), ReadLE4(wav_header + 4)); EXPECT_EQ(static_cast<uint64_t>(file_length - 8), ReadLE4(wav_header + 4));
EXPECT_EQ(static_cast<uint32_t>(data_size + kWavHeaderSize - 8), EXPECT_EQ(static_cast<uint32_t>(data_size + kWavHeaderSize - 8),
ReadLE4(wav_header + 4)); ReadLE4(wav_header + 4));
......
...@@ -258,6 +258,8 @@ size_t File::GetLength() { ...@@ -258,6 +258,8 @@ size_t File::GetLength() {
DCHECK(base_file_.IsValid()); DCHECK(base_file_.IsValid());
int64_t len = base_file_.GetLength(); int64_t len = base_file_.GetLength();
if (len < 0)
return 0;
if (len > static_cast<int64_t>(std::numeric_limits<uint32_t>::max())) if (len > static_cast<int64_t>(std::numeric_limits<uint32_t>::max()))
return std::numeric_limits<uint32_t>::max(); return std::numeric_limits<uint32_t>::max();
......
...@@ -139,6 +139,8 @@ size_t File::GetLength() { ...@@ -139,6 +139,8 @@ size_t File::GetLength() {
DCHECK(base_file_.IsValid()); DCHECK(base_file_.IsValid());
int64_t len = base_file_.GetLength(); int64_t len = base_file_.GetLength();
if (len < 0)
return 0;
if (len > static_cast<int64_t>(std::numeric_limits<uint32_t>::max())) if (len > static_cast<int64_t>(std::numeric_limits<uint32_t>::max()))
return std::numeric_limits<uint32_t>::max(); return std::numeric_limits<uint32_t>::max();
......
...@@ -37,8 +37,10 @@ bool RemoveKeySHA256FromEntry(const std::string& key, ...@@ -37,8 +37,10 @@ bool RemoveKeySHA256FromEntry(const std::string& key,
File entry_file(entry_file_path, flags); File entry_file(entry_file_path, flags);
if (!entry_file.IsValid()) if (!entry_file.IsValid())
return false; return false;
int file_length = entry_file.GetLength(); int64_t file_length = entry_file.GetLength();
SimpleFileEOF eof_record; SimpleFileEOF eof_record;
if (file_length < static_cast<int64_t>(sizeof(eof_record)))
return false;
if (entry_file.Read(file_length - sizeof(eof_record), if (entry_file.Read(file_length - sizeof(eof_record),
reinterpret_cast<char*>(&eof_record), reinterpret_cast<char*>(&eof_record),
sizeof(eof_record)) != sizeof(eof_record)) { sizeof(eof_record)) != sizeof(eof_record)) {
...@@ -73,8 +75,10 @@ bool CorruptKeySHA256FromEntry(const std::string& key, ...@@ -73,8 +75,10 @@ bool CorruptKeySHA256FromEntry(const std::string& key,
File entry_file(entry_file_path, flags); File entry_file(entry_file_path, flags);
if (!entry_file.IsValid()) if (!entry_file.IsValid())
return false; return false;
int file_length = entry_file.GetLength(); int64_t file_length = entry_file.GetLength();
SimpleFileEOF eof_record; SimpleFileEOF eof_record;
if (file_length < static_cast<int64_t>(sizeof(eof_record)))
return false;
if (entry_file.Read(file_length - sizeof(eof_record), if (entry_file.Read(file_length - sizeof(eof_record),
reinterpret_cast<char*>(&eof_record), reinterpret_cast<char*>(&eof_record),
sizeof(eof_record)) != sizeof(eof_record)) { sizeof(eof_record)) != sizeof(eof_record)) {
...@@ -105,8 +109,10 @@ bool CorruptStream0LengthFromEntry(const std::string& key, ...@@ -105,8 +109,10 @@ bool CorruptStream0LengthFromEntry(const std::string& key,
File entry_file(entry_file_path, flags); File entry_file(entry_file_path, flags);
if (!entry_file.IsValid()) if (!entry_file.IsValid())
return false; return false;
int file_length = entry_file.GetLength(); int64_t file_length = entry_file.GetLength();
SimpleFileEOF eof_record; SimpleFileEOF eof_record;
if (file_length < static_cast<int64_t>(sizeof(eof_record)))
return false;
if (entry_file.Read(file_length - sizeof(eof_record), if (entry_file.Read(file_length - sizeof(eof_record),
reinterpret_cast<char*>(&eof_record), reinterpret_cast<char*>(&eof_record),
sizeof(eof_record)) != sizeof(eof_record)) { sizeof(eof_record)) != sizeof(eof_record)) {
......
...@@ -1536,6 +1536,8 @@ SBOX_TESTS_COMMAND int CheckWin10FontLoad(int argc, wchar_t** argv) { ...@@ -1536,6 +1536,8 @@ SBOX_TESTS_COMMAND int CheckWin10FontLoad(int argc, wchar_t** argv) {
std::vector<char> font_data; std::vector<char> font_data;
int64_t len = file.GetLength(); int64_t len = file.GetLength();
if (len < 0)
return SBOX_TEST_NOT_FOUND;
font_data.resize(len); font_data.resize(len);
int read = file.Read(0, &font_data[0], len); int read = file.Read(0, &font_data[0], len);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment