Commit 643f77a8 authored by Robert Sesek's avatar Robert Sesek Committed by Commit Bot

Fix heap overflow in safe_browsing::dmg::HFSBTreeIterator::Next().

The code was not properly checking that the length of the key string
contained within the buffer. The string uses UTF-16 (two-byte) units
but was treating the length as single-byte units. This switches the
code to use the size-checked GetLeafData<T> wrapper.

Bug: 783914
Change-Id: I4f9207532181656e4865eb4c397cae5745b5785a
Reviewed-on: https://chromium-review.googlesource.com/767408Reviewed-by: default avatarJialiu Lin <jialiul@chromium.org>
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516109}
parent 6aa1c94a
......@@ -519,26 +519,25 @@ bool HFSBTreeIterator::Next() {
auto parent_id = OSSwapBigToHostInt32(*GetLeafData<uint32_t>());
auto key_string_length = OSSwapBigToHostInt16(*GetLeafData<uint16_t>());
size_t key_string_end_offset = 0;
if (!base::CheckAdd(current_leaf_offset_, key_string_length)
.AssignIfValid(&key_string_end_offset) ||
key_string_end_offset > leaf_data_.size()) {
DLOG(ERROR) << "Key string length larger than leaf data";
return false;
}
auto* key_string =
reinterpret_cast<uint16_t*>(&leaf_data_[current_leaf_offset_]);
for (uint16_t i = 0;
i < key_string_length;
++i, current_leaf_offset_ += sizeof(uint16_t)) {
key_string[i] = OSSwapBigToHostInt16(key_string[i]);
// Read and byte-swap the variable-length key string.
base::string16 key(key_string_length, '\0');
for (uint16_t i = 0; i < key_string_length; ++i) {
auto* character = GetLeafData<uint16_t>();
if (!character) {
DLOG(ERROR) << "Key string length points past leaf data";
return false;
}
key[i] = OSSwapBigToHostInt16(*character);
}
base::string16 key(key_string, key_string_length);
// Read the record type and then rewind as the field is part of the catalog
// structure that is read next.
current_record_.record_type = OSSwapBigToHostInt16(*GetLeafData<int16_t>());
auto* record_type = GetLeafData<int16_t>();
if (!record_type) {
DLOG(ERROR) << "Failed to read record type";
return false;
}
current_record_.record_type = OSSwapBigToHostInt16(*record_type);
current_record_.unexported = false;
current_leaf_offset_ -= sizeof(int16_t);
switch (current_record_.record_type) {
......
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