Fix integer wraparound in Buffer bounds checks on 32-bit platforms
The typed Read methods (ReadU8, ReadU16, ReadU24, ReadU32, ReadTag, ReadR64) used bounds checks of the form `offset_ + N > length_`. On 32-bit platforms where size_t is 32 bits, if offset_ is close to SIZE_MAX (e.g. 0xFFFFFFFF), the addition `offset_ + N` wraps around to a small value, bypassing the bounds check and allowing out-of-bounds reads from the underlying buffer. Fix by restructuring the checks to `length_ < N || offset_ > length_ - N`, which avoids any arithmetic that could overflow. This is consistent with the existing Read(uint8_t*, size_t) method which already uses the safe `offset_ > length_ - n_bytes` pattern. Also add bounds validation to set_offset() (changing return type from void to bool) to reject offsets beyond the buffer length, and update the sole caller in font.cc to check the return value. This prevents TTC font collection offsets from positioning the buffer past its end.
This commit is contained in:
committed by
Garret Rieger
parent
1f184d0556
commit
1c69169e9e
+13
-7
@@ -82,7 +82,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
inline bool ReadU8(uint8_t *value) {
|
||||
if (offset_ + 1 > length_) {
|
||||
if (length_ < 1 || offset_ > length_ - 1) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
*value = buffer_[offset_];
|
||||
@@ -91,7 +91,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
bool ReadU16(uint16_t *value) {
|
||||
if (offset_ + 2 > length_) {
|
||||
if (length_ < 2 || offset_ > length_ - 2) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
std::memcpy(value, buffer_ + offset_, sizeof(uint16_t));
|
||||
@@ -105,7 +105,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
bool ReadU24(uint32_t *value) {
|
||||
if (offset_ + 3 > length_) {
|
||||
if (length_ < 3 || offset_ > length_ - 3) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
*value = static_cast<uint32_t>(buffer_[offset_]) << 16 |
|
||||
@@ -116,7 +116,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
bool ReadU32(uint32_t *value) {
|
||||
if (offset_ + 4 > length_) {
|
||||
if (length_ < 4 || offset_ > length_ - 4) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
|
||||
@@ -130,7 +130,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
bool ReadTag(uint32_t *value) {
|
||||
if (offset_ + 4 > length_) {
|
||||
if (length_ < 4 || offset_ > length_ - 4) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
|
||||
@@ -139,7 +139,7 @@ class Buffer {
|
||||
}
|
||||
|
||||
bool ReadR64(uint64_t *value) {
|
||||
if (offset_ + 8 > length_) {
|
||||
if (length_ < 8 || offset_ > length_ - 8) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
std::memcpy(value, buffer_ + offset_, sizeof(uint64_t));
|
||||
@@ -151,7 +151,13 @@ class Buffer {
|
||||
size_t offset() const { return offset_; }
|
||||
size_t length() const { return length_; }
|
||||
|
||||
void set_offset(size_t newoffset) { offset_ = newoffset; }
|
||||
bool set_offset(size_t newoffset) {
|
||||
if (newoffset > length_) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
offset_ = newoffset;
|
||||
return true;
|
||||
}
|
||||
|
||||
private:
|
||||
const uint8_t * const buffer_;
|
||||
|
||||
+3
-1
@@ -155,7 +155,9 @@ bool ReadTrueTypeCollection(Buffer* file, const uint8_t* data, size_t len,
|
||||
|
||||
std::map<uint32_t, Font::Table*> all_tables;
|
||||
for (const auto offset : offsets) {
|
||||
file->set_offset(offset);
|
||||
if (!file->set_offset(offset)) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
}
|
||||
Font& font = *font_it++;
|
||||
if (!ReadCollectionFont(file, data, len, &font, &all_tables)) {
|
||||
return FONT_COMPRESSION_FAILURE();
|
||||
|
||||
Reference in New Issue
Block a user