Closed Bug 580233 Opened 10 years ago Closed 10 years ago

Invalid values in GPOS table leading to 'division by zero' [@hb_sanitize_context_t::check_array]

Categories

(Core :: Graphics, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: posidron, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Attached file testcase
Build identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; en-US; rv:2.0b2pre) Gecko/20100718 Minefield/4.0b2pre

Table: GPOS
Table Offset: 0x11c
Value Offset: +0x5d
Values: ff c4 40 0f

Load the provided html file
Attached file callstack
Assignee: nobody → jfkthame
Attachment #458660 - Flags: review?(jdaggett)
blocking2.0: --- → ?
blocking2.0: ? → final+
Hmmm, isn't record_size = 0 and len > 0 an overflow condition?  Not sure if that's possible to get but it seems like you might allow access to the first element of an array in that case, which would be out-of-bounds.
(In reply to comment #3)
> Hmmm, isn't record_size = 0 and len > 0 an overflow condition? 

Not in the sense of this check; the "overflows" flag here relates to the computation of (record_size * len) that's going to be done when calling check_range(), which could otherwise suffer integer overflow if the values were wild.

> Not sure if
> that's possible to get but it seems like you might allow access to the first
> element of an array in that case, which would be out-of-bounds.

If record_size = 0, the array elements are nonexistent, in which case any "access" to them is an error elsewhere. More specifically, the case of record_size = 0 arises in GPOS ValueRecords in the (degenerate) case where NONE of the bits in ValueFormat was set, indicating that none of the optional fields is in fact present. So in that case the GPOS code should not be attempting to look at any of the optional fields.
Attachment #458955 - Flags: review?(jdaggett)
Attachment #458660 - Flags: review?(jdaggett) → review+
Attachment #458955 - Flags: review?(jdaggett) → review+
Pushed patch + crashtest:
http://hg.mozilla.org/mozilla-central/rev/0f5fc40c6a0f

(This patch has also landed in upstream harfbuzz-ng.)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.