ubsan: Don’t call v[0] on empty vectors

Calling std::vector<>::operator[]() with an out-of-range index argument
is undefined behavior. In two cases, Crashpad used &v[0] in situations
where it was known that the address would not be used. These calls were
wrapped in conditions guarding against vector emptiness.

While s[0] is valid on an empty string, in two cases, Crashpad used
&s[0] as an argument to a system call that would be a no-op. These calls
were wrapped in similar conditions to avoid the system call.

The two uses of vector with undefined behavior were caught by the
following tests in crashpad_snapshot_test with
UndefinedBehaviorSanitizer:

[ RUN      ] CrashpadInfoClientOptions.OneModule
/Users/mark/compilatorium/llvm.build/bin/../include/c++/v1/vector:1493:12:
runtime error: reference binding to null pointer of type
'crashpad::process_types::section'
[       OK ] CrashpadInfoClientOptions.OneModule (72 ms)

[ RUN      ] ProcessSnapshotMinidump.Empty
/Users/mark/compilatorium/llvm.build/bin/../include/c++/v1/vector:1493:12:
runtime error: reference binding to null pointer of type
'MINIDUMP_DIRECTORY'
[       OK ] ProcessSnapshotMinidump.Empty (1 ms)

The Crashpad codebase was audited by searching for resize() calls and
analyzing how resized strings and vectors are used.

TEST=*
BUG=
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/1283243004 .
This commit is contained in:
Mark Mentovai 2015-08-20 11:50:19 -04:00
parent 5064aeb784
commit 34aef02cc7
4 changed files with 15 additions and 11 deletions

View File

@ -95,7 +95,8 @@ bool MachOImageSegmentReader::Initialize(ProcessReader* process_reader,
}
sections_.resize(segment_command_.nsects);
if (!process_types::section::ReadArrayInto(
if (!sections_.empty() &&
!process_types::section::ReadArrayInto(
process_reader,
load_command_address + segment_command_.Size(),
segment_command_.nsects,

View File

@ -65,7 +65,8 @@ bool ProcessSnapshotMinidump::Initialize(FileReaderInterface* file_reader) {
}
stream_directory_.resize(header_.NumberOfStreams);
if (!file_reader_->ReadExactly(
if (!stream_directory_.empty() &&
!file_reader_->ReadExactly(
&stream_directory_[0],
header_.NumberOfStreams * sizeof(stream_directory_[0]))) {
return false;

View File

@ -41,6 +41,7 @@ XattrStatus ReadXattr(const base::FilePath& file,
// Resize the buffer and read into it.
value->resize(buffer_size);
if (!value->empty()) {
ssize_t bytes_read = getxattr(file.value().c_str(), name.data(),
&(*value)[0], value->size(),
0, 0);
@ -49,6 +50,7 @@ XattrStatus ReadXattr(const base::FilePath& file,
return XattrStatus::kOtherError;
}
DCHECK_EQ(bytes_read, buffer_size);
}
return XattrStatus::kOK;
}

View File

@ -319,8 +319,8 @@ void ChildPortHandshake::RunClientInternal_ReadPipe(int pipe_read,
DCHECK_LT(service_name_length,
implicit_cast<uint32_t>(BOOTSTRAP_MAX_NAME_LEN));
if (service_name_length > 0) {
service_name->resize(service_name_length);
if (!service_name->empty()) {
CheckedReadFile(pipe_read, &(*service_name)[0], service_name_length);
}
}