Closed Bug 775228 (CVE-2012-5830) Opened 12 years ago Closed 12 years ago

use-after-free when loading html file on osx

Categories

(Core :: Graphics, defect)

Product:
Core
Component:
Graphics
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla19
Tracking Flags:
Tracking Status
firefox16 + wontfix
firefox17 + verified
firefox18 + verified
firefox19 --- verified
firefox-esr10 17+ fixed
firefox-esr17 --- fixed

People

(Reporter: miaubiz, Assigned: mattwoodrow)

Details

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

Attachments

(6 files, 1 obsolete file)

the repro
723 bytes, text/html
Details
asan log from osx trunk
15.31 KB, text/plain
Details
osx stable crashwrangler log
14.06 KB, text/plain
Details
osx aurora crashwrangler log
24.67 KB, text/plain
Details
Check the surface status before returning it.
1.05 KB, patch
joe
: review+
bajaj
: approval-mozilla-aurora+
lsblakk
: approval-mozilla-beta+
Details | Diff | Splinter Review
Patch that applies to esr10
1.04 KB, patch
akeybl
: approval-mozilla-esr10+
Details | Diff | Splinter Review
Attached file repro, asan log, crashwrangler logs. (obsolete) —
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
Attached file the repro —
Attachment #643510 - Attachment mime type: text/plain → text/html
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
(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? :)
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
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
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+
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
Closed: 12 years ago
status-firefox19: --- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
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)
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)
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.
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+
(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.
Attachment #680685 - Flags: approval-mozilla-esr10?
Attachment #680685 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: