Closed Bug 790879 (CVE-2012-5835) Opened 12 years ago Closed 12 years ago

integer overflow, invalid write w/webgl bufferdata

Categories

(Core :: Graphics: CanvasWebGL, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox16 --- wontfix
firefox17 --- fixed
firefox18 --- fixed
firefox-esr10 17+ fixed
firefox-esr17 --- fixed

People

(Reporter: miaubiz, Assigned: bjacob)

Details

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

Attachments

(5 files, 1 obsolete file)

Attached file repro
I load: <html> <head> <script> gl=document.createElement('canvas').getContext('experimental-webgl') var buf = gl.createBuffer() gl.bindBuffer(gl.ARRAY_BUFFER, buf) var magic = 0x12345678 gl.bufferData(gl.ARRAY_BUFFER, new Uint8Array(magic+1), gl.STATIC_DRAW) gl.bufferData(gl.ARRAY_BUFFER, Math.pow(2, 32), gl.STATIC_DRAW) gl.bufferSubData(gl.ARRAY_BUFFER, magic, new Uint8Array(1)) </script> </head> </html> and I get: exception=EXC_BAD_ACCESS:signal=11:is_exploitable=yes:instruction_disassembly=movb %al,(%rdi):instruction_address=0x00007fff92c82a41:access_type=write:access_address=0x0000000012345678: Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x0000000012345678 Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 libsystem_c.dylib 0x00007fff92c82a41 memmove$VARIANT$sse42 + 57 1 GLEngine 0x000000010cfa9982 glBufferSubData_Exec + 856 rdi: 0x0000000012345678 r11: 0x0000000012345678 r12: 0x0000000012345678 ASAN:SIGSEGV ==3966== ERROR: AddressSanitizer crashed on unknown address 0x000012345678 (pc 0x7fff92c82a41 sp 0x7fff5fbf5380 bp 0x7fff5fbf5380 T0) AddressSanitizer can not provide additional info. ABORTING #0 0x7fff92c82a40 in memmove$VARIANT$sse42 (in libsystem_c.dylib) + 227 #1 0x100009291 in wrap_memmove (in firefox) + 545 #2 0x13add8981 in glDeleteBuffers_Exec (in GLEngine) + 1727 #3 0x105d41d4a in mozilla::gl::GLContext::fBufferSubData(unsigned int, long, long, void const*) (in XUL) + 90 #4 0x105d25b2f in mozilla::WebGLContext::BufferSubData(unsigned int, long long, mozilla::dom::TypedArray_base<unsigned char, &(JS_GetObjectAsArrayBufferView(JSContext*, JSObject*, unsigned int*, unsigned char**))>&) (in XUL) + 575
Attached file asan log osx
Attached patch work around Mac bufferData bug (obsolete) — Splinter Review
The type of the size parameter to bufferData is like size_t. So on 64bit it is valid to pass up to 2^64-1. Apparently the Mac GL lib doesn't know that. This patch works around it by rejecting bufferData calls with size exceeding UINT32_MAX before passing them to the GL.
Attachment #660823 - Flags: review?
Actually, this type is signed.
Attachment #660823 - Attachment is obsolete: true
Attachment #660823 - Flags: review?
Attachment #660893 - Flags: review?
Benoit, what security rating would you give this issue?
Attachment #660893 - Flags: review? → review?(jgilbert)
Attachment #660893 - Flags: review?(jgilbert) → review+
(In reply to Al Billings [:abillings] from comment #6) > Benoit, what security rating would you give this issue? Random memory can be access, but only for read, not for write or execute (for all we know). So I would naturally call this sec-high (information leakage vulnerability) but looking at https://wiki.mozilla.org/Security_Severity_Ratings, I see that "Any crash where random memory is accessed" is sec-criticial, so I guess sec-critical then (but I would like to understand the rationale for this rule). Up to you.
Assignee: nobody → bjacob
Target Milestone: --- → mozilla18
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
@benoit: why is it only a read? doesn't it end up calling fBufferSubData() which will attempt to memmove stuff into bad places? as in logs in c#0
miaubiz, I can't remember at all what I was thinking then. Comment 0 does say "access_type=write" so it seems I was just wrong. That explains why it's sec-critical then.
Keywords: sec-highsec-critical
Keywords: sec-vector
CC'ing Google.
Comment on attachment 660893 [details] [diff] [review] work around Mac bufferData bug Sorry for the delayed request. [Approval Request Comment] Bug caused by (feature/regressing bug #): driver bug User impact if declined: sec-critical Testing completed (on m-c, etc.): been on m-c since last week Risk to taking this patch (and alternatives if risky): no risk String or UUID changes made by this patch: none
Attachment #660893 - Flags: approval-mozilla-beta?
Attachment #660893 - Flags: approval-mozilla-aurora?
Comment on attachment 660893 [details] [diff] [review] work around Mac bufferData bug This missed our final beta, and doesn't seem critical for release. Approving for Aurora 17, however.
Attachment #660893 - Flags: approval-mozilla-beta?
Attachment #660893 - Flags: approval-mozilla-beta-
Attachment #660893 - Flags: approval-mozilla-aurora?
Attachment #660893 - Flags: approval-mozilla-aurora+
Is the ESR branch affected, given this is a driver bug?
Most certainly yes.
Keywords: verifyme
(In reply to Benoit Jacob [:bjacob] from comment #17) > Most certainly yes. Please prepare a patch for mozilla-esr10 as well then. Thanks!
Attached patch esr10 patchSplinter Review
Here is the typical ESR10 user: http://www.grg.org/images/JCalment/JCalmentDate3.jpg I content that she doesn't care about WebGL and would be better off if we simply turned it off in ESR, which would also relieve us from a significant maintenance burden. Anyway, here is the ESR10 patch for this bug. Note that we didn't have GLContext::WorkAroundDriverBugs() in ESR10.
Attachment #675624 - Flags: review?(jgilbert)
Attachment #675624 - Flags: review?(jgilbert) → review+
Comment on attachment 675624 [details] [diff] [review] esr10 patch [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: sec:critical Fix Landed on Version: 17+ Risk to taking this patch (and alternatives if risky): no risk. very simple, well tested already. String or UUID changes made by this patch: none
Attachment #675624 - Flags: approval-mozilla-esr10?
Attachment #675624 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Whiteboard: [adv-track-main17+]
Comment on attachment 675624 [details] [diff] [review] esr10 patch Funny story - esr10 doesn't define GenerateWarning. Backed out for build failures. https://hg.mozilla.org/releases/mozilla-esr10/rev/7ded7f96bbbe
Attachment #675624 - Attachment is obsolete: true
Flags: in-testsuite?
Attachment #675624 - Attachment is obsolete: false
Whiteboard: [adv-track-main17+] → [adv-track-main17+][adv-track-esr17+]
Alias: CVE-2012-5835
Whiteboard: [adv-track-main17+][adv-track-esr17+] → [adv-track-main17+][adv-track-esr17+][asan]
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: