Closed Bug 802778 (CVE-2012-5838) Opened 12 years ago Closed 12 years ago

crash in copyTexImage2D with image dimensions too large for given level

Categories

(Core :: Graphics: CanvasWebGL, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox16 --- wontfix
firefox17 + fixed
firefox18 + fixed
firefox19 + fixed
firefox-esr10 --- unaffected
firefox-esr17 --- fixed

People

(Reporter: miaubiz, Assigned: bjacob)

Details

(Keywords: sec-critical, sec-vector, Whiteboard: [adv-track-main17+][adv-track-esr17-][asan] band-aid for Mesa driver bug)

Attachments

(3 files)

Attached file repro case —
similar to https://bugzilla.mozilla.org/show_bug.cgi?id=785734

on intel copyTexImage2D with a level of 9 or 10 and width and height that when multiplied come close to 2<<level, causes crashes,

==3424== ERROR: AddressSanitizer crashed on unknown address 0x0c0800010000 (pc 0x7f40953a8d69 sp 0x7fff9cd4cf28 bp 0x7f409b106080 T0)
==3455== ERROR: AddressSanitizer crashed on unknown address 0x000000000219 (pc 0x7f4410ea8009 sp 0x7fff39b3a900 bp 0x7f4416c21c80 T0)
==3484== ERROR: AddressSanitizer crashed on unknown address 0x300400008000 (pc 0x7f2f643a8d69 sp 0x7fffc23d1648 bp 0x7f2f6a921480 T0)
==3673== ERROR: AddressSanitizer crashed on unknown address 0x060a00008000 (pc 0x7f552b0a8d69 sp 0x7fff0c6814a8 bp 0x7f5530e07480 T0)
==3703== ERROR: AddressSanitizer crashed on unknown address 0x0c0800008000 (pc 0x7f3ee3da8d69 sp 0x7fffc3351cc8 bp 0x7f3ee9b22080 T0)
==3893== ERROR: AddressSanitizer crashed on unknown address 0x000100000251 (pc 0x7f1e3f2a6526 sp 0x7fff9cf2a070 bp 0x7f1e3f550140 T0)
Attached file asan log linux —
Attachment #672474 - Attachment mime type: text/plain → text/html
Those aren't very helpful logs, why aren't we getting a stack? Maybe this will repro in debug builds.

On a mac ASan nightly I just get some "invalid drawable" messages so could be a driver issue like bug 785734
Assignee: nobody → bjacob
Is this bug on Mac (as this bug attributes indicate) or on Linux (as comment 1 suggests)?

Can you share your about:support Graphics information?
(In reply to miaubiz from comment #0)
> Created attachment 672474 [details]
> repro case
> 
> similar to https://bugzilla.mozilla.org/show_bug.cgi?id=785734
> 
> on intel copyTexImage2D with a level of 9 or 10 and width and height that
> when multiplied come close to 2<<level, causes crashes,

In the testcase, all calls satisfy an even stronger condition:

  max(width, height) << level > max_texture_size

Indeed, the smallest we have is the last call, which has height 64 and level 9, and

  64 << 9 = 2^15.

This means that the fix we applied in bug 785734 should block this... except that that validation function is only called by texImage2D and not by copyTexImage2D. That seems to be the bug here, patch coming.
Attachment #674753 - Flags: review?
Summary: crash with mipmaps → crash in copyTexImage2D with image dimensions too large for given level
Attachment #674753 - Flags: review? → review?(jgilbert)
@bjacob: linux, with intel driver. (I reported on a mac, and didn't fix it manually)
OS: Mac OS X → Linux
This bug in Mesa was fixed by:

commit 771e7b6d884bb4294a89f276a904d90b28efb90a
Author: Brian Paul <brianp@vmware.com>
Date:   Tue Sep 4 20:17:15 2012 -0600

    mesa: fix per-level max texture size error checking

It's in Mesa's main branch and the 9.0 release.  I just cherry-picked it back to the Mesa 8.0 branch so it'll appear in Mesa 8.0.6 (if we actually do a 8.0.6 release).
Thanks! We'll still do the work-around as this one is regular validation that we mean to do ourselves anyway before calling the GL.
Benjoit, can you suggest a security rating for this?
It is the same as bug 785734 (except for a different function) so it has to be sec-critical as well.
Attachment #674753 - Flags: review?(jgilbert) → review+
Critsmash triage: Does this affect ESR 10 and other Firefox versions? How far back does this go?
It certainly affects every Firefox version since we shipped WebGL i.e. Firefox 4.
Let's try to get this fixed on all branches (Central/Aurora/Beta/ESR10) as soon as possible (no later than Monday), given this appears to be a low risk sg:crit fix.
I would appear that I forgot to land this :-(

https://hg.mozilla.org/integration/mozilla-inbound/rev/8f0f5a7da4bc
Target Milestone: --- → mozilla19
Comment on attachment 674753 [details] [diff] [review]
validate copyTex[Sub]Image2D too

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version:
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: 

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Attachment #674753 - Flags: approval-mozilla-esr10?
Attachment #674753 - Flags: approval-mozilla-beta?
Attachment #674753 - Flags: approval-mozilla-aurora?
Erm, approval request comment: trivial fix for critical sec bug.
Comment on attachment 674753 [details] [diff] [review]
validate copyTex[Sub]Image2D too

Thanks for noms, please land asap to get into the final Beta on Monday.
Attachment #674753 - Flags: approval-mozilla-esr10?
Attachment #674753 - Flags: approval-mozilla-esr10+
Attachment #674753 - Flags: approval-mozilla-beta?
Attachment #674753 - Flags: approval-mozilla-beta+
Attachment #674753 - Flags: approval-mozilla-aurora?
Attachment #674753 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/8f0f5a7da4bc
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-aurora/rev/1e01edd1a7a1
https://hg.mozilla.org/releases/mozilla-beta/rev/24d7bd351f8d

I attempted to land this on esr10, but had to back it out for build failures.

/builds/slave/m-esr10-osx64/build/content/canvas/src/WebGLContextGL.cpp: In member function 'GLenum mozilla::WebGLContext::CheckedBufferData(GLenum, GLsizeiptr, const GLvoid*, GLenum)':
/builds/slave/m-esr10-osx64/build/content/canvas/src/WebGLContextGL.cpp:441: warning: comparison is always false due to limited range of data type
/builds/slave/m-esr10-osx64/build/content/canvas/src/WebGLContextGL.cpp: In member function 'nsresult mozilla::WebGLContext::CopyTexSubImage2D_base(WebGLenum, WebGLint, WebGLenum, WebGLint, WebGLint, WebGLint, WebGLint, WebGLsizei, WebGLsizei, bool)':
/builds/slave/m-esr10-osx64/build/content/canvas/src/WebGLContextGL.cpp:848: error: 'ValidateLevelWidthHeightForTarget' was not declared in this scope
/builds/slave/m-esr10-osx64/build/content/canvas/src/WebGLContextGL.cpp:849: error: return-statement with no value, in function returning 'unsigned int'
Indeed, esr10 is significantly different there. Will make a patch.
Keywords: sec-vector
Flags: sec-bounty+
Whiteboard: band-aid for Mesa driver bug
Confirmed the crash on 2012-10-17 nightly using provided bug file.
Cannot reproduce on ESR 10.0.10.
Ubuntu 11.10
(In reply to Matt Wobensmith from comment #22)
> Confirmed the crash on 2012-10-17 nightly using provided bug file.
> Cannot reproduce on ESR 10.0.10.
> Ubuntu 11.10

Thanks Matt!

Given this, can we call ESR10 unaffected here Benoit, or should we continue to pursue a fix for tomorrow?
It's possible that I misremembered and ESR10 is actually unaffected: maybe it did some validation that got removed/broken in a later version. That's the only way I can make sense of Matt's finding. Let's go with that, saves me time :-)
Alias: CVE-2012-5838
Whiteboard: band-aid for Mesa driver bug → [adv-track-main17+] band-aid for Mesa driver bug
Whiteboard: [adv-track-main17+] band-aid for Mesa driver bug → [adv-track-main17+][adv-track-esr17-] band-aid for Mesa driver bug
Keywords: verifyme
Whiteboard: [adv-track-main17+][adv-track-esr17-] band-aid for Mesa driver bug → [adv-track-main17+][adv-track-esr17-][asan] band-aid for Mesa driver bug
Keywords: verifyme
Group: core-security
You need to log in before you can comment on or make changes to this bug.