The default bug view has changed. See this FAQ.
Bug 775228 (CVE-2012-5830)

use-after-free when loading html file on osx

RESOLVED FIXED in Firefox 17

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
4 months ago

People

(Reporter: miaubiz, Assigned: mattwoodrow)

Tracking

(4 keywords)

Trunk
mozilla19
x86
Mac OS X
crash, csectype-uaf, sec-critical, testcase
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox16+ wontfix, firefox17+ verified, firefox18+ verified, firefox19 verified, firefox-esr1017+ fixed, firefox-esr17 fixed)

Details

(Whiteboard: [asan][adv-track-main17+][adv-track-esr17+])

Attachments

(6 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 643508 [details]
repro, asan log, crashwrangler logs.

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_4) AppleWebKit/536.11 (KHTML, like Gecko) Chrome/20.0.1132.57 Safari/536.11

Steps to reproduce:

loaded:


<html>
  <head>
    <style>
      #el0 { -moz-perspective: 1px; } 
      #el1 { -moz-transform: rotateY(1deg); } 
      #el2 { width: 40000; } 
    </style>
    <script> 
      onload = function() {
        el0=document.createElement('div') 
        el0.setAttribute('id','el0') 
        document.body.appendChild(el0) 
        el1=document.createElement('tr') 
        el1.setAttribute('id','el1') 
        el0.appendChild(el1) 
        el2=document.createElement('input') 
        el2.setAttribute('id','el2') 
        el1.appendChild(el2) 
        document.body.offsetTop
        setTimeout(function() {
          window.open(document.location)
        },2000)
      }
    </script>
  </head>
  <body>
  </body>
</html>



Actual results:

asan said:

==18953== ERROR: AddressSanitizer heap-use-after-free on address 0x000131e9d480 at pc 0x106e8cecc bp 0x7fff5fbeead0 sp 0x7fff5fbeeac8
READ of size 8 at 0x000131e9d480 thread T0
    #0 0x106e8cecc in _ZN7mozilla6layersL15PixmanTransformEPK15gfxImageSurfaceS3_RK11gfx3DMatrix8gfxPoint (in XUL) + 764
    #1 0x106e8b1c6 in _ZN7mozilla6layersL11Transform3DEP11gfxASurfaceP10gfxContextRK7gfxRectRK11gfx3DMatrixR8gfxPointb (in XUL) + 1990
    #2 0x106e8a236 in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::ThebesLayer*, gfxContext*, nsIntRegion const&, nsIntRegion const&, void*), void*, mozilla::layers::ReadbackProcessor*) (in XUL) + 5254
    #3 0x106e89e6f in mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void 

0x000131e9d480 is located 0 bytes inside of 72-byte region [0x000131e9d480,0x000131e9d4c8)
freed by thread T0 here:
    #0 0x10000d095 in (anonymous namespace)::mz_free(_malloc_zone_t*, void*) (in firefox) + 85
    #1 0x10000cb10 in wrap_free (in firefox) + 80
    #2 0x106df1a46 in gfxASurface::Release() (in XUL) + 310
    #3 0x106e70557 in gfxQuartzSurface::GetAsImageSurface() (in XUL) + 343
    #4 0x106e8abd2 in 


Expected results:

:D
(Reporter)

Comment 1

5 years ago
Created attachment 643510 [details]
the repro
(Reporter)

Comment 2

5 years ago
Created attachment 643511 [details]
asan log from osx trunk
(Reporter)

Comment 3

5 years ago
Created attachment 643512 [details]
osx stable crashwrangler log
(Reporter)

Updated

5 years ago
Attachment #643510 - Attachment mime type: text/plain → text/html
(Reporter)

Comment 4

5 years ago
Created attachment 643513 [details]
osx aurora crashwrangler log
Attachment #643508 - Attachment is obsolete: true
Component: Untriaged → Graphics
Keywords: testcase
Product: Firefox → Core
Whiteboard: [asan]
Joe what do you think of the asan output in comment 0? (See also other attached logs)
Assignee: nobody → joe
I think it looks like we're using a pixman-transformed surface after we released it!
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: crash, sec-critical
status-firefox17: --- → affected
(In reply to Joe Drew (:JOEDREW!) from comment #6)
> I think it looks like we're using a pixman-transformed surface after we
> released it!

Can you please not do that? :)

Comment 9

5 years ago
looks like we should probably pay a bounty on this.  any second thoughts on that?
Joe, are you working on a fix here?
I'm not, but perhaps Jeff can.
Assignee: joe → jmuizelaar
status-firefox18: --- → affected
tracking-firefox17: --- → +
tracking-firefox18: --- → +
After we find out where the flaw is we need to check how old the bug is -- possibly affects ESR-10.
status-firefox-esr10: --- → ?
tracking-firefox-esr10: --- → ?
Taking off esr10 flags for now until it can be determined if 16 is affected and also sent an email out to ask about how much priority this bug can get in the current cycle.
status-firefox-esr10: ? → ---
tracking-firefox-esr10: ? → ---
tracking-firefox16: --- → ?
Jeff told me he would take a look a this today, tracking for 16 for now until we hear back from him.
tracking-firefox16: ? → +
Looks like we're just trying to make a surface with a dimension greater than 32768. This fails and we don't handle that properly.
Matt, how is the case of very large 3d transformed surfaces supposed to be handled?
I was wondering that. From a few quick tests it appears that large layers just stop rendering ATM on chrome/firefox. Not a great long term solution but for now I'd suggest we continue to do that here.
Assignee: jmuizelaar → matt.woodrow
If we think this is low risk enough to uplift into FF16, please land and nominate for aurora/beta approval prior to Tuesday's Beta 5 go to build. If not, we'll untrack for release.
We've already gone to build with beta6 so the window on 16 landings has closed. Wontfixing for 16.
status-firefox16: --- → wontfix
(Assignee)

Comment 20

5 years ago
Created attachment 674459 [details] [diff] [review]
Check the surface status before returning it.

Looks like a regression from bug 575521, 3d transforms is just one of the few things to hit it.
Attachment #674459 - Flags: review?(joe)
Comment on attachment 674459 [details] [diff] [review]
Check the surface status before returning it.

Review of attachment 674459 [details] [diff] [review]:
-----------------------------------------------------------------

The patch is obviously safe and correct; hopefully it also fixes the bug :)
Attachment #674459 - Flags: review?(joe) → review+
(Assignee)

Comment 22

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a97b903917f
I think this technically needed the "sec-approval" flag to be granted before landing -- security team recently introduced that flag to let them gate when security bugs are & aren't allowed to land, so as to prevent us from zero-day'ing ourselves.

No sense in backing out now, because the patch is now already public (plus, we're probably at a good time in the release cycle for security bugs to land, so it probably would've been granted anyway).  Just mentioning it for future reference.
https://hg.mozilla.org/mozilla-central/rev/1a97b903917f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox19: --- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19

Comment 25

5 years ago
As this is a sec-critical crash that's tracking+ for 17, can we get this uplifted given that it sticks?
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #25)
> As this is a sec-critical crash that's tracking+ for 17, can we get this
> uplifted given that it sticks?

Yep, and we'll need to know whether ESR10 is affected as well.
(In reply to Daniel Holbert [:dholbert] from comment #23)
> I think this technically needed the "sec-approval" flag to be granted before
> landing -- security team recently introduced that flag to let them gate when
> security bugs are & aren't allowed to land, so as to prevent us from
> zero-day'ing ourselves.
> 
> No sense in backing out now, because the patch is now already public (plus,
> we're probably at a good time in the release cycle for security bugs to
> land, so it probably would've been granted anyway).  Just mentioning it for
> future reference.

As a sec-critical bug, yes, it should have had sec-approval? set on the patch when it was ready to go. 

What's done is done now though. It is a learning process.
Matt, needs your attention for uplift.
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 29

5 years ago
Comment on attachment 674459 [details] [diff] [review]
Check the surface status before returning it.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 575521
User impact if declined: Potential security issue.
Testing completed (on m-c, etc.): Been on m-c for a few days
Risk to taking this patch (and alternatives if risky): Incredibly low risk, just checks if the surface is valid before returning it.
String or UUID changes made by this patch: None
Attachment #674459 - Flags: approval-mozilla-aurora?
Flags: needinfo?(matt.woodrow)

Comment 30

5 years ago
This is also marked tracking for 17, do you intend to land this on beta as well? Remember, we don't have much more than a week to comfortably land things on 17 beta as everything should land in time for b5.
Comment on attachment 674459 [details] [diff] [review]
Check the surface status before returning it.

Approving the low risk patch considering the user impact if declined & it being baked for a few days.
Attachment #674459 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Nomination for beta would be great here, let's try to get this into tomorrow's beta 4.
(Assignee)

Comment 33

5 years ago
Comment on attachment 674459 [details] [diff] [review]
Check the surface status before returning it.

Same as above, we really want to get this into beta asap.
Attachment #674459 - Flags: approval-mozilla-beta?
Attachment #674459 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 34

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/114614cc6263
https://hg.mozilla.org/releases/mozilla-beta/rev/b90097a37788
status-firefox17: affected → fixed
status-firefox18: affected → fixed
(In reply to Matt Woodrow (:mattwoodrow) from comment #29)
> Bug caused by (feature/regressing bug #): bug 575521

If this is the case, doesn't this affect ESR 10?
Whiteboard: [asan] → [asan][adv-track-main17+]
(In reply to Al Billings [:abillings] from comment #35)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #29)
> > Bug caused by (feature/regressing bug #): bug 575521
> 
> If this is the case, doesn't this affect ESR 10?

Let's assume yes until told otherwise, given the regressing bug. Matt - please prepare a patch for ESR10 unless we've got this wrong somehow.
status-firefox-esr10: --- → affected
tracking-firefox-esr10: --- → 17+
Matt, any update on the ESR patch? We're running out of time to make the release.
(Assignee)

Comment 38

4 years ago
Created attachment 680685 [details] [diff] [review]
Patch that applies to esr10
Attachment #680685 - Flags: approval-mozilla-esr10?

Updated

4 years ago
Attachment #680685 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
(Assignee)

Comment 39

4 years ago
https://hg.mozilla.org/releases/mozilla-esr10/rev/d51c1344f90c
status-firefox-esr10: affected → fixed
status-firefox-esr17: --- → fixed
Alias: CVE-2012-5830
Whiteboard: [asan][adv-track-main17+] → [asan][adv-track-main17+][adv-track-esr17+]
Keywords: verifyme
Since this bug is more than a month old, I don't have ASan builds for both before and after the patch landed.

However, the crash occurs in daily debug builds as well, and I can confirm that the crash is fixed as of 2012-10-26, after the patch landed.

Confirmed crash on 2012-07-18, m-c
Confirmed fixed on 2012-10-26, m-c
Confirmed fixed on 2013-01-16, beta
Confirmed fixed on 2013-01-16, Aurora
status-firefox17: fixed → verified
status-firefox18: fixed → verified
status-firefox19: fixed → verified
Group: core-security
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Keywords: csectype-uaf
You need to log in before you can comment on or make changes to this bug.