553273 - Freetype2 table 'fvar' mmvar_len signed integer overflow

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Christoph Diehl [:posidron]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; de; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)

Hello,

there could be a potential exploitable signed int overflow while parsing the fvar
table. The following is more theoretical than practical. I believe if we use zlib
compression (WOFF) it could be possible to extend table_len large enough to bypass
the following conditions.

Some compression examples:

>>> from zlib import compress
>>> len(compress("\x00" * 0xffff))
84
>>> len(compress("\x00" * 0xffffff))
16316
>>> len(compress("\x00" * 0xfffffff))
260922
>>> len(compress("\x00" * 0xffffffff))
???

mmvar_len must overflow back to >= 0. In order to produce such a large value we
have to juggle with instanceSize and axisCount. 

Conditions
----------

679       if ( fvar_head.version != (FT_Long)0x00010000L                      ||
680            fvar_head.countSizePairs != 2                                  ||
681            fvar_head.axisSize != 20                                       ||

682            fvar_head.instanceSize != 4 + 4 * fvar_head.axisCount          ||
683            fvar_head.offsetToData + fvar_head.axisCount * 20U +
684              fvar_head.instanceCount * fvar_head.instanceSize > table_len )
685       {
686         error = TT_Err_Invalid_Table;
687         goto Exit;
688       }

signed int overflow
-------------------

693       /* XXX: TODO - check for overflows */
694       face->blend->mmvar_len =
695         sizeof ( FT_MM_Var ) +
696         fvar_head.axisCount * sizeof ( FT_Var_Axis ) +
697         fvar_head.instanceCount * sizeof ( FT_Var_Named_Style ) +
698         fvar_head.instanceCount * fvar_head.axisCount * sizeof ( FT_Fixed ) +
699         5 * fvar_head.axisCount;



Thanks for your time.


Reproducible: Couldn't Reproduce

Comment 1

[Christoph Diehl [:posidron]]

Edit:

URL:

Comment 2

[Reed Loden [:reed]]

Have you reported this issue upstream to The Freetype Project? If not, that's fine, as we can do it for you, but just want to make sure we're not duplicating efforts to investigate this.

Comment 3

[Christoph Diehl [:posidron]]

No, i did not. Thank you.

Comment 4

[Christoph Diehl [:posidron]]

Attachment: Calculates mmvar_len

If I am correct then following values are needed to overflow mmvar_len.

mmvar_len: 147419 (overflowed >= 0)

table_len: 4294705192 (uncompressed)
axisCount: 16382
instanceCount: 65531
instanceSize: 65532

Comment 5

[Daniel Veditz [:dveditz]]

http://mxr.mozilla.org/mozilla-central/source/modules/freetype2/src/truetype/ttgxvar.c#693

> 693       /* XXX: TODO - check for overflows */
> 694       face->blend->mmvar_len =

*Sigh*

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

Is this the same as bug 553433?

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Oh, no, it isn't - nevermind.

Comment 8

[Reed Loden [:reed]]

I spoke with Werner Lemberg (FreeType developer) out-of-band, and he pointed me towards George Williams, the author of GX font support in FreeType. I've contacted George to hopefully get his assistance with this issue.

Comment 9

[Jonathan Kew [:jfkthame]]

We should upgrade freetype to resolve bug 553433, then consider this issue.

(Is there any way to trigger the GX-variations code within Gecko, I wonder? We might not be exposed to this in practice - but I'm not sure at the moment.)

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: patch, v1: check values will not lead to arith overflow

Although axisCount and instanceCount are specified as unsigned 16-bit values in the fvar table, there are actually additional constraints on their range:

(1) if axisCount > 0x3ffe, then instanceSize would overflow and therefore the table would be invalid;

(2) because each instance has a (unique) name ID, and the valid range of IDs is from 256 to 32767, instanceCount cannot legally exceed 0x7eff.

AFAICT, once these two constraints are checked, it is no longer possible for the calculation of mmvar_len to overflow 32-bit arithmetic.

Werner, if this looks sensible to you, could you also take this upstream to freetype?

Comment 11

[Werner Lemberg]

Pushed to git, thanks.

Comment 12

[Reed Loden [:reed]]

(In reply to comment #11)
> Pushed to git, thanks.

http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/?id=25e742c573e3b88e5a4e342733f1836466628ff8

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 437256 [details] [diff] [review]
patch, v1: check values will not lead to arith overflow

taking the fact that this landed upstream as an implicit r+.

One nit, please add a readme note

Comment 14

[Jonathan Kew [:jfkthame]]

pushed: http://hg.mozilla.org/mozilla-central/rev/64ebf70ed4a2

sorry, commit message gives the wrong bug number (553272) :(