743475 - (CVE-2012-0473) WebGLBuffer::FindMaxUshortElement passes wrong template arguments to FindMaxElementInSubArray

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Matias Juntunen (:dwarfcrank)]

Attachment: Fix the wrong template parameter in call to WebGLBuffer::FindMaxElementInSubArray

WebGLBuffer::FindMaxUshortElement in content/canvas/src/WebGLContext.h passes GLshort instead of GLushort to FindMaxElementInSubArray, even though only GLubyte or GLushort should be used.

I've attached a patch to fix the issue.

Comment 1

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a4ef59d7490

Comment 2

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/8a4ef59d7490

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 613142 [details] [diff] [review]
Fix the wrong template parameter in call to WebGLBuffer::FindMaxElementInSubArray

[Approval Request Comment]
Regression caused by (bug #): we've had this bug ever since we ship WebGL
User impact if declined: security risk, will explain once this bug is hidden
Testing completed (on m-c, etc.): just landed
Risk to taking this patch (and alternatives if risky): trivial fix, no risk
String changes made by this patch:

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

OK, now that this bug is hidden, here's the explanation.

WebGL.drawElements validates that all the indices are in range. It does that by computing the max index, and comparing that to the min buffer size.

This bug causes the max index computation to be wrong. Basically, indices >= 32768 are ignored by the max element computation.

An attacker could exploit this by drawing with indices all in the range 32768--65535. This would allow them to read some illegal video memory, whence the sec-high rating.

Comment 5

[Daniel Veditz [:dveditz]]

If it's landing on beta/Firefox 12 it also needs to land on the ESR branch

Comment 7

[Alex Keybl [:akeybl]]

Comment on attachment 613142 [details] [diff] [review]
Fix the wrong template parameter in call to WebGLBuffer::FindMaxElementInSubArray

[Triage Comment]
Approved for Beta 12 and Aurora 13 given the low risk evaluation and sg:high rating. Please land asap.

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/144d327b35a9
http://hg.mozilla.org/releases/mozilla-beta/rev/7b1f6af2b2d7

Comment 9

[Al Billings [:abillings - ex-MoCo]]

Is there a means to test this fix for verification purposes?

Comment 12

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Al Billings [:abillings] from comment #9)
> Is there a means to test this fix for verification purposes?

I'll whip a quick testcase.

Comment 13

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: Testcase

Comment 14

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 613142 [details] [diff] [review]
Fix the wrong template parameter in call to WebGLBuffer::FindMaxElementInSubArray

Please go ahead and land as per https://wiki.mozilla.org/Release_Management/ESR_Landing_Process

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Verified fixed in trunk with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20120411 Firefox/14.0a1 and Benoit's nice testcase.

Comment 16

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/releases/mozilla-esr10/rev/76d2e9680f28

Comment 17

[Raymond Forbes[:rforbes]]

rforbes-bugspam-for-setting-that-bounty-flag-20130719