Closed Bug 553273 Opened 14 years ago Closed 14 years ago

Freetype2 table 'fvar' mmvar_len signed integer overflow

Categories

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

x86_64
Windows Vista
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: posidron, Assigned: jfkthame)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [sg:critical?])

Attachments

(2 files)

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
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.
No, i did not. Thank you.
Attached file 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
Component: Graphics → Layout: Text
QA Contact: thebes → layout.fonts-and-text
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*
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [sg:critical?]
Oh, no, it isn't - nevermind.
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.
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.)
Depends on: CVE-2009-0946
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?
Assignee: nobody → jfkthame
Attachment #437256 - Flags: review?(wl)
Pushed to git, thanks.
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
Attachment #437256 - Flags: review?(wl) → review+
pushed: http://hg.mozilla.org/mozilla-central/rev/64ebf70ed4a2

sorry, commit message gives the wrong bug number (553272) :(
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Group: core-security
You need to log in before you can comment on or make changes to this bug.