Bugzilla

`OpenTypeSTAT::Parse()` (`gfx\ots\src\stat.cc`) can read beyond bounds if a font header contains specially-crafted values for `designAxisSize`, `designAxisCount`, and `designAxisOffset`. The bug can be reached only on 32-bit platforms, and it appears to be latent -- Searchfox indicates that `OpenTypeSTAT::Parse()` is not called anywhere, though it is built into the Firefox executable.

The bug is on lines 62-68 (FIREFOX_121_0_RELEASE):

```
 31: bool OpenTypeSTAT::Parse(const uint8_t* data, size_t length) {
 32:   Buffer table(data, length);
 33:   if (!table.ReadU16(&this->majorVersion) ||
 34:       !table.ReadU16(&this->minorVersion) ||
 35:       !table.ReadU16(&this->designAxisSize) ||
 36:       !table.ReadU16(&this->designAxisCount) ||
 37:       !table.ReadU32(&this->designAxesOffset) ||
 38:       !table.ReadU16(&this->axisValueCount) ||
 39:       !table.ReadU32(&this->offsetToAxisValueOffsets) ||
 40:       !(this->minorVersion < 1 || table.ReadU16(&this->elidedFallbackNameID))) {
 41:     return Drop("Failed to read table header");
 42:   }
 ...
 51:   if (this->designAxisSize < sizeof(AxisRecord)) {
 52:     return Drop("Invalid designAxisSize");
 53:   }
 54: 
 55:   size_t headerEnd = table.offset();
 56: 
 57:   if (this->designAxisCount == 0) {
 58:     if (this->designAxesOffset != 0) {
 59:       Warning("Unexpected non-zero designAxesOffset");
 60:       this->designAxesOffset = 0;
 61:     }
 62:   } else {
 63:     if (this->designAxesOffset < headerEnd ||
 64:         size_t(this->designAxesOffset) +
 65:           size_t(this->designAxisCount) * size_t(this->designAxisSize) > length) {
 66:       return Drop("Invalid designAxesOffset");
 67:     }
 68:   }
```

The issue is that the calculation on lines 64-5 can overflow on 32-bit builds, because in those builds, `size_t` is 32 bits. So, for example, if `designAxesOffset` is `0xffffffff`, `designAxisCount` is `16`, and `designAxisSize` is `8`, the calculation yields `0x7f` in 32 bits. This would then pass the length check on line 65. Once that occurs, line 71 sets `table.offset_` to `0xffffffff`. The `table.Readxxx()` functions on lines 74-6 then read from `buffer_ + 0xffffffff`, then wrap back to `0`...:

```
 69: 
 70:   for (size_t i = 0; i < this->designAxisCount; i++) {
 71:     table.set_offset(this->designAxesOffset + i * this->designAxisSize);
 72:     this->designAxes.emplace_back();
 73:     auto& axis = this->designAxes[i];
 74:     if (!table.ReadU32(&axis.axisTag) ||
 75:         !table.ReadU16(&axis.axisNameID) ||
 76:         !table.ReadU16(&axis.axisOrdering)) {
 77:       return Drop("Failed to read design axis");
 78:     }
 79:     if (!CheckTag(axis.axisTag)) {
 80:       return Drop("Bad design axis tag");
 81:     }
 82:     if (!ValidateNameId(axis.axisNameID)) {
 83:       return true;
 84:     }
 85:   }
```

because the `ReadUxxx()` functions' checks also overflow, e.g., `ReadU32()` (`gfx/ots/src/ots.h`) does:

```
 135: bool ReadU32(uint32_t *value) {
 136:   if (offset_ + 4 > length_) {
 137:     return OTS_FAILURE();
 138:   }
 139:   std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
 140:   *value = ots_ntohl(*value);
 141:   offset_ += 4;
 142:   return true;
 143: }
```

where `offset_` isa `size_t`, so lines 136 and 141 overflow from `0xffffffff` to `3` on the first call to `ReadU32()`, passing the check on lines 136-38 and corrupting the value of `offset` on line 141.

`OpenTypeSTAT::Parse()` (`gfx\ots\src\stat.cc`) can read beyond bounds if a font header contains specially-crafted values for `designAxisSize`, `designAxisCount`, and `designAxisOffset`. The bug can be reached only on 32-bit platforms, and it appears to be latent -- Searchfox indicates that `OpenTypeSTAT::Parse()` is not called anywhere, though it is built into the Firefox executable.

The bug is on lines 62-68 (FIREFOX_121_0_RELEASE):

```
 31: bool OpenTypeSTAT::Parse(const uint8_t* data, size_t length) {
 32:   Buffer table(data, length);
 33:   if (!table.ReadU16(&this->majorVersion) ||
 34:       !table.ReadU16(&this->minorVersion) ||
 35:       !table.ReadU16(&this->designAxisSize) ||
 36:       !table.ReadU16(&this->designAxisCount) ||
 37:       !table.ReadU32(&this->designAxesOffset) ||
 38:       !table.ReadU16(&this->axisValueCount) ||
 39:       !table.ReadU32(&this->offsetToAxisValueOffsets) ||
 40:       !(this->minorVersion < 1 || table.ReadU16(&this->elidedFallbackNameID))) {
 41:     return Drop("Failed to read table header");
 42:   }
 ...
 51:   if (this->designAxisSize < sizeof(AxisRecord)) {
 52:     return Drop("Invalid designAxisSize");
 53:   }
 54: 
 55:   size_t headerEnd = table.offset();
 56: 
 57:   if (this->designAxisCount == 0) {
 58:     if (this->designAxesOffset != 0) {
 59:       Warning("Unexpected non-zero designAxesOffset");
 60:       this->designAxesOffset = 0;
 61:     }
 62:   } else {
 63:     if (this->designAxesOffset < headerEnd ||
 64:         size_t(this->designAxesOffset) +
 65:           size_t(this->designAxisCount) * size_t(this->designAxisSize) > length) {
 66:       return Drop("Invalid designAxesOffset");
 67:     }
 68:   }
```

The issue is that the calculation on lines 64-5 can overflow on 32-bit builds, because in those builds, `size_t` is 32 bits. So, for example, if `designAxesOffset` is `0xffffffff`, `designAxisCount` is `16`, and `designAxisSize` is `8`, the calculation yields `0x7f` in 32 bits. This would then pass the length check on line 65. Once that occurs, line 71 sets `table.offset_` to `0xffffffff`. The `table.Readxxx()` functions on lines 74-6 then read from `buffer_ + 0xffffffff`, then wrap back to `0`...:

```
 69: 
 70:   for (size_t i = 0; i < this->designAxisCount; i++) {
 71:     table.set_offset(this->designAxesOffset + i * this->designAxisSize);
 72:     this->designAxes.emplace_back();
 73:     auto& axis = this->designAxes[i];
 74:     if (!table.ReadU32(&axis.axisTag) ||
 75:         !table.ReadU16(&axis.axisNameID) ||
 76:         !table.ReadU16(&axis.axisOrdering)) {
 77:       return Drop("Failed to read design axis");
 78:     }
 79:     if (!CheckTag(axis.axisTag)) {
 80:       return Drop("Bad design axis tag");
 81:     }
 82:     if (!ValidateNameId(axis.axisNameID)) {
 83:       return true;
 84:     }
 85:   }
```

because the `ReadUxxx()` functions' checks also overflow, e.g., `ReadU32()` (`gfx/ots/src/ots.h`) does:

```
 135: bool ReadU32(uint32_t *value) {
 136:   if (offset_ + 4 > length_) {
 137:     return OTS_FAILURE();
 138:   }
 139:   std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
 140:   *value = ots_ntohl(*value);
 141:   offset_ += 4;
 142:   return true;
 143: }
```

where `offset_` isa `size_t`, so lines 136 and 141 overflow from `0xffffffff` to `3` on the first call to `ReadU32()`, passing the check on lines 136-38 and corrupting the value of `offset` on line 141.

There appears also to be a similar bug involving `offsetToAxisValueOffsets` in the same code.