Bug 744888 (CVE-2012-3105)

Work around NVIDIA driver bug in glBufferData

RESOLVED FIXED in Firefox 13

Status

()

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

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

({sec-vector})

unspecified
mozilla14
sec-vector
Points:
---

Firefox Tracking Flags

(firefox12 wontfix, firefox13 fixed, firefox14 fixed, firefox-esr1013+ fixed)

Details

(Whiteboard: [qa-][advisory-tracking+])

Attachments

(4 attachments)

Thanks to Ken/Google for letting us know about this. My understanding of

  http://code.google.com/p/chromium/issues/detail?id=118970

is that the NVIDIA drivers have a bug: glBufferData with null data doesn't actually allocate the buffer until actual data is uploaded to it.

This affects WebGL. We could work around this in 2 different places in WebGL (in WebGL bufferData and in the attrib 0 emulation code), but instead, I think that this is best handled at the level of GLContext.
(Assignee)

Comment 1

5 years ago
Created attachment 614571 [details] [diff] [review]
work around nvidia bufferdata bug
Attachment #614571 - Flags: review?(jgilbert)
Attachment #614571 - Flags: review?(jgilbert) → review+
Patch looks fine, and should fix the problem, but it probably fixes it too conservatively. This forces us to allocate memory, zero, and upload a buffer of the full size when what is being asked for is merely buffer allocation. If we're just trying to assure the buffer is allocated and sized properly, we should just be able to upload one byte at the very end of the buffer.
Comment on attachment 614571 [details] [diff] [review]
work around nvidia bufferdata bug

Review of attachment 614571 [details] [diff] [review]:
-----------------------------------------------------------------

Moving to r- pending discussion and/or clarification.
Attachment #614571 - Flags: review+ → review-

Comment 4

5 years ago
Note that when Chrome's GPU subsystem receives a glBufferData call from an untrusted client with NULL as the input, it explicitly allocates a large zeroed buffer and uploads it to the GPU. See GLES2DecoderImpl::DoBufferData, http://src.chromium.org/viewvc/chrome/trunk/src/gpu/command_buffer/service/gles2_cmd_decoder.cc?view=markup .
(Assignee)

Comment 5

5 years ago
Ken: thanks, the notion a 'trusted client' at the level of the GL wrapper is quite interesting. Maybe we should do the same.
webgl.bufferData(null) is disallowed, but I don't think we care otherwise. I think the important issue here is just to be sure buffers are allocated when we ask them to be. Ideally, we would just defer this until we (maybe) need them, and (maybe) assure allocation then. (ideally warning if we are using uninitialized buffers, though it should only help us internally, and we shouldn't be having any problems there)

I need to read through the chromium bug again when I'm more coherent.

After re-reading it anyways, I think I understand better. Part of the root of the problem is that the driver 'consumes'(reads) all enabled vertex arrays, even if the in-use program does not read from them. (Which I was certainly not aware of) If a too-short (or in this driver's case, lazily-thus-not-yet-allocated) buffer is bound to an enabled vertex pointer, the driver can be accessing uninitialized/invalid memory.

I think I'll need to read through our implementation and trace this out for myself.
(Assignee)

Comment 7

5 years ago
Created attachment 614867 [details] [diff] [review]
like this instead?
Attachment #614867 - Flags: review?(jgilbert)
Comment on attachment 614867 [details] [diff] [review]
like this instead?

Review of attachment 614867 [details] [diff] [review]:
-----------------------------------------------------------------

This is what seems like a more minimal change, to me.

::: gfx/gl/GLContext.h
@@ +2014,5 @@
> +        if (WorkAroundDriverBugs() &&
> +            !data &&
> +            Vendor() == VendorNVIDIA)
> +        {
> +            char c;

You might want to initialize this to keep everything happy about uninitialized variables being used. (Not that it should matter in practice, but still)
Attachment #614867 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 9

5 years ago
http://hg.mozilla.org/mozilla-central/rev/e74b044c18b8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Comment on attachment 614867 [details] [diff] [review]
like this instead?

[Approval Request Comment]
Regression caused by (bug #): We've had this problem since WebGL shipped in Firefox 4
User impact if declined: security issue: information leakage vulnerability with NVIDIA driver
Testing completed (on m-c, etc.): been on m-c for a few days.
Risk to taking this patch (and alternatives if risky): Does not seem risky. Could be a tiny performance overhead. Could fail to fix the problem. If we run into another unforeseen driver bug, it could even lead to a crash, but we don't have any definite reason to think that such a driver bug is waiting for us.
String changes made by this patch: none
Attachment #614867 - Flags: approval-mozilla-beta?
Attachment #614867 - Flags: approval-mozilla-aurora?
Comment on attachment 614867 [details] [diff] [review]
like this instead?

[Triage Comment]
We weighed the risk and reward of this nomination and decided that this <sg:high issue was not worth landing between beta 6 and our final build. Approving for Aurora 13, however.
Attachment #614867 - Flags: approval-mozilla-beta?
Attachment #614867 - Flags: approval-mozilla-beta-
Attachment #614867 - Flags: approval-mozilla-aurora?
Attachment #614867 - Flags: approval-mozilla-aurora+

Updated

5 years ago
status-firefox-esr10: --- → affected
tracking-firefox-esr10: --- → 13+
http://hg.mozilla.org/releases/mozilla-aurora/rev/9edd931bf07f
status-firefox-esr10: affected → fixed
(Assignee)

Updated

5 years ago
status-firefox-esr10: fixed → affected
(Assignee)

Updated

5 years ago
status-firefox12: --- → wontfix
status-firefox13: --- → fixed
status-firefox14: --- → fixed
bustage fix:
http://hg.mozilla.org/releases/mozilla-aurora/rev/f53d4a882978
Is there a testcase QA can use to verify this fix?
Whiteboard: [qa?]
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #14)
> Is there a testcase QA can use to verify this fix?

Bump.
The original Chromium bug has a testcase:
http://code.google.com/p/chromium/issues/detail?id=118970
Thanks Benoit.
Whiteboard: [qa?] → [qa+]
Assigning to Benoit to get this nominated, and landed to ESR.
Assignee: nobody → bjacob
Comment on attachment 614867 [details] [diff] [review]
like this instead?

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Well, this bug allows exploiting an information leakage vulnerability in NVIDIA drivers, that would probably rate sg:high
User impact if declined: information leakage vulnerability with NVIDIA driver
Fix Landed on Version: 14
Risk to taking this patch (and alternatives if risky): low risk (see above aurora approval request)
String changes made by this patch: none
Attachment #614867 - Flags: approval-mozilla-esr10?
Anthony, do you have a copy of the testcase? I have no access to http://code.google.com/p/chromium/issues/detail?id=118970.
Created attachment 623837 [details]
CanvasMatrix.js to use with the testcase
Created attachment 623838 [details]
testcase: unknown-2.html

Here's the testcase. The chromium bug says:

REPRODUCTION CASE
1. Download both files (nothing wrong with CanvasMatrix.js)
2. chrome-asan unknown-2.html
3. gpu process crashes
Comment on attachment 614867 [details] [diff] [review]
like this instead?

low risk, sg:high and fix already on mozilla-beta (13), approving for ESR landing.
Attachment #614867 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Running this testcase against a Firefox 12 on my Win 7 x64 box, which has an nvdia crash, I don't have any issues. Does this only reproduce on certain operating systems or in ASAN builds (of which we have none on Windows)?
(In reply to Lukas Blakk [:lsblakk] from comment #23)
> low risk, sg:high and fix already on mozilla-beta (13), approving for ESR
> landing.

If this is sg:high, why isn't it marked as such?
(In reply to Al Billings [:abillings] from comment #25)
> (In reply to Lukas Blakk [:lsblakk] from comment #23)
> > low risk, sg:high and fix already on mozilla-beta (13), approving for ESR
> > landing.
> 
> If this is sg:high, why isn't it marked as such?

Maybe my views on what's sg:high aren't universally shared, or maybe it's just an oversight.

Patch ready for landing on ESR10 but tree is closed due to bug 756365.
Keywords: sec-vector
https://hg.mozilla.org/releases/mozilla-esr10/rev/d740253129c7
status-firefox-esr10: affected → fixed
Whiteboard: [qa+] → [qa+][advisory-tracking+]
I don't crash on Firefox 12 with the following NVIDIA driver/hardware so I am unable to verify this fix. Is there specific driver versions that are affected or hardware mentioned in the Chrome bug?

  Application Basics
        Name
        Firefox

        Version
        12.0

        User Agent
        Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0

  Graphics

        Adapter Description
        NVIDIA GeForce GT 240M

        Vendor ID
        0x10de

        Device ID
        0x0a34

        Adapter RAM
        1024

        Adapter Drivers
        nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um

        Driver Version
        8.17.13.142

        Driver Date
        5-15-2012

        Direct2D Enabled
        true

        DirectWrite Enabled
        true (6.1.7601.17789)

        ClearType Parameters
        DISPLAY1 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 100 Enhanced Contrast: 400 ] DISPLAY2 [ Gamma: 2200 Pixel Structure: RGB ClearType Level: 50 Enhanced Contrast: 200 ]

        WebGL Renderer
        Google Inc. -- ANGLE (NVIDIA GeForce GT 240M) -- OpenGL ES 2.0 (ANGLE 1.0.0.963)

        GPU Accelerated Windows
        1/1 Direct3D 10

        AzureBackend
        direct2d
Benoit, this bug seems to be unreproducible on our end. Do you have any leads QA can pursue to test this fix? 

If there is someone else watching this bug who is able to test this, please help us out by verifying it is fixed. QA is seeking verification in Firefox 13, 10.0.5ESR and 14.0a2.

Thank you.
It's alright, driver bugs are often tricky to reproduce. If you tried and couldn't reproduce, it's probably OK to stop there on this bug.
Marking [qa-] since QA is unable to reproduce the original issue. Please mark [qa+] is a more reproducible case is found so we can verify this is fixed.

Thanks
Whiteboard: [qa+][advisory-tracking+] → [qa-][advisory-tracking+]
Group: core-security
Alias: CVE-2012-3105
You need to log in before you can comment on or make changes to this bug.