Last Comment Bug 775228 - (CVE-2012-5830) use-after-free when loading html file on osx
(CVE-2012-5830)
: use-after-free when loading html file on osx
Status: RESOLVED FIXED
[asan][adv-track-main17+][adv-track-e...
: crash, sec-critical, testcase
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla19
Assigned To: Matt Woodrow (:mattwoodrow)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-18 12:28 PDT by miaubiz
Modified: 2014-07-24 13:43 PDT (History)
17 users (show)
rforbes: sec‑bounty+
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
wontfix
+
verified
+
verified
verified
17+
fixed
fixed


Attachments
repro, asan log, crashwrangler logs. (9.19 KB, application/octet-stream)
2012-07-18 12:28 PDT, miaubiz
no flags Details
the repro (723 bytes, text/html)
2012-07-18 12:28 PDT, miaubiz
no flags Details
asan log from osx trunk (15.31 KB, text/plain)
2012-07-18 12:29 PDT, miaubiz
no flags Details
osx stable crashwrangler log (14.06 KB, text/plain)
2012-07-18 12:29 PDT, miaubiz
no flags Details
osx aurora crashwrangler log (24.67 KB, text/plain)
2012-07-18 12:30 PDT, miaubiz
no flags Details
Check the surface status before returning it. (1.05 KB, patch)
2012-10-23 17:11 PDT, Matt Woodrow (:mattwoodrow)
joe: review+
bajaj.bhavana: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Review
Patch that applies to esr10 (1.04 KB, patch)
2012-11-12 10:08 PST, Matt Woodrow (:mattwoodrow)
akeybl: approval‑mozilla‑esr10+
Details | Diff | Review

Description miaubiz 2012-07-18 12:28:24 PDT
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
Comment 1 miaubiz 2012-07-18 12:28:59 PDT
Created attachment 643510 [details]
the repro
Comment 2 miaubiz 2012-07-18 12:29:24 PDT
Created attachment 643511 [details]
asan log from osx trunk
Comment 3 miaubiz 2012-07-18 12:29:59 PDT
Created attachment 643512 [details]
osx stable crashwrangler log
Comment 4 miaubiz 2012-07-18 12:30:58 PDT
Created attachment 643513 [details]
osx aurora crashwrangler log
Comment 5 David Bolter [:davidb] 2012-07-25 11:09:55 PDT
Joe what do you think of the asan output in comment 0? (See also other attached logs)
Comment 6 Joe Drew (not getting mail) 2012-07-25 12:07:39 PDT
I think it looks like we're using a pixman-transformed surface after we released it!
Comment 8 David Bolter [:davidb] 2012-08-23 13:24:17 PDT
(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 chris hofmann 2012-08-27 13:52:44 PDT
looks like we should probably pay a bounty on this.  any second thoughts on that?
Comment 10 Al Billings [:abillings] 2012-08-30 13:11:05 PDT
Joe, are you working on a fix here?
Comment 11 Joe Drew (not getting mail) 2012-08-30 13:17:03 PDT
I'm not, but perhaps Jeff can.
Comment 12 Daniel Veditz [:dveditz] 2012-09-06 13:48:15 PDT
After we find out where the flaw is we need to check how old the bug is -- possibly affects ESR-10.
Comment 13 Lukas Blakk [:lsblakk] use ?needinfo 2012-09-06 16:15:41 PDT
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.
Comment 14 Lukas Blakk [:lsblakk] use ?needinfo 2012-09-10 15:40:55 PDT
Jeff told me he would take a look a this today, tracking for 16 for now until we hear back from him.
Comment 15 Jeff Muizelaar [:jrmuizel] 2012-09-11 16:52:20 PDT
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.
Comment 16 Jeff Muizelaar [:jrmuizel] 2012-09-11 16:55:15 PDT
Matt, how is the case of very large 3d transformed surfaces supposed to be handled?
Comment 17 Benoit Girard (:BenWa) 2012-09-12 03:17:14 PDT
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.
Comment 18 Alex Keybl [:akeybl] 2012-09-19 17:38:06 PDT
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.
Comment 19 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-03 10:43:11 PDT
We've already gone to build with beta6 so the window on 16 landings has closed. Wontfixing for 16.
Comment 20 Matt Woodrow (:mattwoodrow) 2012-10-23 17:11:28 PDT
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.
Comment 21 Joe Drew (not getting mail) 2012-10-24 07:27:26 PDT
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 :)
Comment 22 Matt Woodrow (:mattwoodrow) 2012-10-24 16:06:30 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a97b903917f
Comment 23 Daniel Holbert [:dholbert] 2012-10-24 16:22:42 PDT
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.
Comment 24 Ryan VanderMeulen [:RyanVM] 2012-10-25 05:42:03 PDT
https://hg.mozilla.org/mozilla-central/rev/1a97b903917f
Comment 25 Robert Kaiser (not working on stability any more) 2012-10-25 14:30:00 PDT
As this is a sec-critical crash that's tracking+ for 17, can we get this uplifted given that it sticks?
Comment 26 Alex Keybl [:akeybl] 2012-10-25 16:15:08 PDT
(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.
Comment 27 Al Billings [:abillings] 2012-10-25 18:01:06 PDT
(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.
Comment 28 Joe Drew (not getting mail) 2012-10-26 12:40:38 PDT
Matt, needs your attention for uplift.
Comment 29 Matt Woodrow (:mattwoodrow) 2012-10-26 18:13:19 PDT
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
Comment 30 Robert Kaiser (not working on stability any more) 2012-10-27 07:46:13 PDT
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 31 bhavana bajaj [:bajaj] 2012-10-29 12:00:13 PDT
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.
Comment 32 Lukas Blakk [:lsblakk] use ?needinfo 2012-10-29 14:47:22 PDT
Nomination for beta would be great here, let's try to get this into tomorrow's beta 4.
Comment 33 Matt Woodrow (:mattwoodrow) 2012-10-29 14:58:26 PDT
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.
Comment 35 Al Billings [:abillings] 2012-10-31 16:20:13 PDT
(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?
Comment 36 Alex Keybl [:akeybl] 2012-11-01 16:11:08 PDT
(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.
Comment 37 Al Billings [:abillings] 2012-11-07 16:28:56 PST
Matt, any update on the ESR patch? We're running out of time to make the release.
Comment 38 Matt Woodrow (:mattwoodrow) 2012-11-12 10:08:23 PST
Created attachment 680685 [details] [diff] [review]
Patch that applies to esr10
Comment 39 Matt Woodrow (:mattwoodrow) 2012-11-13 11:48:35 PST
https://hg.mozilla.org/releases/mozilla-esr10/rev/d51c1344f90c
Comment 40 Matt Wobensmith [:mwobensmith][:matt:] 2013-01-16 15:27:54 PST
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
Comment 41 Raymond Forbes[:rforbes] 2013-07-19 18:25:55 PDT
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Comment 42 Tracy Walker [:tracy] 2014-01-10 10:40:41 PST
mass remove verifyme requests greater than 4 months old

Note You need to log in before you can comment on or make changes to this bug.