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

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: 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

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1 - avoid potential divide-by-zero in hb sanitizer

Comment 3

[John Daggett (:jtd)]

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.

Comment 4

[Jonathan Kew [:jfkthame]]

(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.

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: add the testcase as a crashtest

Comment 6

[Jonathan Kew [:jfkthame]]

Pushed patch + crashtest:
http://hg.mozilla.org/mozilla-central/rev/0f5fc40c6a0f

(This patch has also landed in upstream harfbuzz-ng.)