Bug 552216 (CVE-2010-1028)

WOFF heap corruption due to integer overflow

RESOLVED FIXED in mozilla1.9.3a4

Status

()

Core
Layout: Text
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: bsterne, Assigned: jfkthame)

Tracking

({crash, testcase, verified1.9.2})

Trunk
mozilla1.9.3a4
x86
All
crash, testcase, verified1.9.2
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(blocking1.9.2 .2+, status1.9.2 .2-fixed, status1.9.1 unaffected)

Details

(Whiteboard: [sg:critical?])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Created attachment 432357 [details]
Proof of concept

Evgeny L. from Vulndisco reported a crash due to a buffer overflow in our WOFF parser.  I am able to reproduce the crash, but the stack I get doesn't appear to be in font parser code.  I'll try it in a debug build shortly to see if it looks any different.  Here's the crash report where it looks like we're calling null:
0bc35855-ee77-4c61-b469-c7c482100313

To reproduce, run the attached python script ff1.py and load
http://localhost:8080/1.html
(Reporter)

Updated

8 years ago
OS: Mac OS X → All
Whiteboard: [sg:critical?]
(Reporter)

Comment 1

8 years ago
More useful link to crash report:
http://crash-stats.mozilla.com/report/index/0bc35855-ee77-4c61-b469-c7c482100313
On windows I see bp-37919069-18f3-4289-aa5d-33b6a2100313 which makes a little more sense -- at least the crash address looks like it came from the bogus server data.
Also slightly different bp-4119d51b-c798-4c23-82f4-7aa652100313
(Assignee)

Updated

8 years ago
Assignee: nobody → jfkthame
Attachment #432363 - Flags: review?(jdaggett)

Comment 5

8 years ago
we should try and squeeze this in to fx 3.6.2.

We need to confirm but it's possible this is the zeroday reported by Evgeny and Secunia at http://secunia.com/advisories/38608/
blocking1.9.1: --- → ?
(Assignee)

Comment 6

8 years ago
Switching the blocking request to 1.9.2, woff is not supported in 1.9.1 (Fx 3.5).
blocking1.9.1: ? → ---
blocking1.9.2: --- → ?

Comment 7

8 years ago
I wonder what is causing a few crash reports with the same signature as in comment 2 to show up for fx3.5.8

nsCOMPtr_base::assign_assuming_AddRef(nsISupports*) | nsInputStreamPump::OnStateStop()

http://crash-stats.mozilla.com/report/index/b1ab0b18-0e4a-4545-8be4-382692100301

http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=nsCOMPtr_base%3A%3Aassign_assuming_AddRef%28nsISupports*%29%20|%20nsInputStreamPump%3A%3AOnStateStop%28%29&date=&range_value=8&range_unit=weeks&process_type=all&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&signature=nsCOMPtr_base%3A%3Aassign_assuming_AddRef%28nsISupports*%29%20|%20nsInputStreamPump%3A%3AOnStateStop%28%29


there are also some 3.5.8 and 3.5.3 reports on the signature in comment 3

nsStreamListenerTee::OnStopRequest(nsIRequest*, nsISupports*, unsigned int)


http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=nsStreamListenerTee%3A%3AOnStopRequest%28nsIRequest*%2C%20nsISupports*%2C%20unsigned%20int%29&date=&range_value=2&range_unit=weeks&process_type=all&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&signature=nsStreamListenerTee%3A%3AOnStopRequest%28nsIRequest*%2C%20nsISupports*%2C%20unsigned%20int%29
(Assignee)

Comment 8

8 years ago
Note that none of these crash signatures are closely associated with the woff-decoder bug (even though on 3.6, they may be caused by it). The bug is such that we end up using zlib to decompress into a too-small buffer; we therefore overwrite some other chunk of memory with the decomressed data; and subsequently crash when another part of the code makes use of its now-damaged data. So similar crash signatures could occur as a result of more or less any memory-stomping bug.

So all this tells us is that there must be something in 3.5.x that can trash memory in a vaguely similar way. It's certainly not the woff code in that case, as it isn't even present in the 1.9.1 tree. Maybe there's some other compressed-format decoder such as jpeg or png that can be fooled in a similar way? If we had a proof-of-concept that triggers the crashes on 3.5.x we could probably locate the problem there.
Do we need to allow orig length anywhere near 4G-3? Even given machines that can handle it that's a ridiculous font size -- no reasonable font will be anything close to that by several orders of magnitude and it allows an unreasonable font to suck up huge amounts of your resources. On OS X 10.5 the biggest font I have is 华文细黑.ttf at 17.4 Mb, followed by AppleGothic at 15. Half of 'em are under 1Mb.
status1.9.1: --- → unaffected
status1.9.2: --- → wanted
Summary: Investigate reported WOFF parser buffer overflow → WOFF heap corruption due to integer overflow
CC'ing the reporter

Comment 11

8 years ago
(In reply to comment #5)
> we should try and squeeze this in to fx 3.6.2.
> 
> We need to confirm but it's possible this is the zeroday reported by Evgeny and
> Secunia at http://secunia.com/advisories/38608/

Exactly, this is the bug i've found.
(Assignee)

Comment 12

8 years ago
(In reply to comment #9)
> Do we need to allow orig length anywhere near 4G-3? 

Well, of course that's a ridiculous size; I'm not sure how we would pick an appropriate smaller limit, though. And if orig length of an individual table is that large, the file as a whole will fail sanityCheck a moment later as the total size will be excessive.
Comment on attachment 432363 [details] [diff] [review]
check for potential arithmetic overflow

This patch is OK but I'd prefer a patch that just made offs, orig and comp all uint64_t. That's a lot more robust.
Attachment #432363 - Flags: review?(jdaggett) → review+
(Assignee)

Comment 14

8 years ago
Created attachment 432405 [details] [diff] [review]
use 64-bit arithmetic to avoid risk of overflow
Attachment #432405 - Flags: review?(roc)
Attachment #432405 - Flags: review?(roc) → review+
(Assignee)

Comment 15

8 years ago
Pushed:
 http://hg.mozilla.org/mozilla-central/rev/aaf4b4f00941
plus Windows bustage fix:
 http://hg.mozilla.org/mozilla-central/rev/30267b65a901
(Assignee)

Updated

8 years ago
Attachment #432363 - Attachment is obsolete: true
(Assignee)

Comment 16

8 years ago
Created attachment 432408 [details] [diff] [review]
patch for 1.9.2 branch, includes followup windows bustage fix
Attachment #432408 - Flags: approval1.9.2.3?
Attachment #432408 - Flags: approval1.9.2.2?
(Assignee)

Comment 17

8 years ago
Argh, additional bustage fix:
http://hg.mozilla.org/mozilla-central/rev/ba99b3a279cf
(Assignee)

Comment 18

8 years ago
Created attachment 432409 [details] [diff] [review]
patch for 1.9.2 branch, includes followups
Attachment #432408 - Attachment is obsolete: true
Attachment #432409 - Flags: approval1.9.2.3?
Attachment #432409 - Flags: approval1.9.2.2?
Attachment #432408 - Flags: approval1.9.2.3?
Attachment #432408 - Flags: approval1.9.2.2?
Created attachment 432419 [details] [diff]
crashtest.diff

Here's a crashtest.  Please land after the fix has shipped on the 1.9.2 branch.

Updated

8 years ago
Flags: in-testsuite?
Comment on attachment 432409 [details] [diff] [review]
patch for 1.9.2 branch, includes followups

a1922=beltzner, please land on mozilla1.9.2 immediately and follow up here with changeset.

Evgeny, thanks for confirming that this is the bug.
Attachment #432409 - Flags: approval1.9.2.3?
Attachment #432409 - Flags: approval1.9.2.2?
Attachment #432409 - Flags: approval1.9.2.2+
(marking blocking - also, is this now FIXED on trunk?)
blocking1.9.2: ? → .2+
(Assignee)

Comment 22

8 years ago
Pushed to 1.9.2
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/827a6883442f
Status: NEW → RESOLVED
Last Resolved: 8 years ago
status1.9.2: wanted → .2-fixed
Resolution: --- → FIXED

Comment 23

8 years ago
Does this apply to 1.9.1 branch as well? Might not be good to ship this fix on 1.9.2 while leaving 1.9.1 vulnerable, if so.
(Assignee)

Comment 24

8 years ago
1.9.1 is unaffected (see flags above).
Since this is a security fix, shouldn't the patch be super-reviewed, as per our review policy (http://www.mozilla.org/hacking/reviewers.html)?

Updated

8 years ago
Keywords: crash, testcase
I verified the problem and the fix with the attached PoC using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2pre) Gecko/20100315 Namoroka/3.6.2pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.2pre) Gecko/20100315 Namoroka/3.6.2pre (.NET CLR 3.5.30729).
Keywords: verified1.9.2
Alias: CVE-2010-1028
Attachment #432405 - Flags: superreview+
Comment on attachment 432409 [details] [diff] [review]
patch for 1.9.2 branch, includes followups

sr=dveditz (for reed). this patch had many many eyes on it before it landed (including mine) but if a formal marker makes people happy then here it is.
Attachment #432409 - Flags: superreview+
Duplicate of this bug: 554189
Group: core-security
Target Milestone: --- → mozilla1.9.3a4

Comment 30

8 years ago
(In reply to comment #27)
> (From update of attachment 432409 [details] [diff] [review])
> sr=dveditz (for reed). this patch had many many eyes on it before it landed
> (including mine) but if a formal marker makes people happy then here it is.

So many that they all failed to notice the obvious flaw below before it landed (fixed in comment 17)?

-    if (tableTotal > 0xffffffffU - orig) {
+    tableTotal += orig;
+    if (tableTotal > 0xffffffffU) {
       return eWOFF_invalid;
     }
     tableTotal += orig;

Comment 31

8 years ago
It could be patched much more easy without inc of a bit's:

uint32_t tableTotal = 0;
//...
uint32_t orig = READ32BE(dirEntry->origLen);
//...
#define MAX(x,y) ((x)>(y)?(x):(y))
if ( tableTotal + orig < MAX(tableTotal,orig) )
{
  return eWOFF_invalid;
}
else
{
  tableTotal += orig;
}

Comment 32

8 years ago
Any remaining issues suggested in comment 7 and 8 are now being tracked in  Bug 555139 -  (CVE-2010-1122)
verified FIXED on build:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.1) Gecko/20100330 Namoroka/3.6.1 Fennec/1.0.1
You need to log in before you can comment on or make changes to this bug.