802778 - (CVE-2012-5838) crash in copyTexImage2D with image dimensions too large for given level

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[miaubiz]

Attachment: 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)

Comment 1

[miaubiz]

Attachment: asan log linux

Comment 3

[Daniel Veditz [:dveditz]]

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

Comment 4

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

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?

Comment 5

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

(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.

Comment 6

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

Attachment: validate copyTex[Sub]Image2D too

Comment 7

[miaubiz]

@bjacob: linux, with intel driver. (I reported on a mac, and didn't fix it manually)

Comment 8

[Brian Paul]

This bug in Mesa was fixed by:

commit 771e7b6d884bb4294a89f276a904d90b28efb90a
Author: Brian Paul <[e-mail]>
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).

Comment 9

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

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.

Comment 10

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

Benjoit, can you suggest a security rating for this?

Comment 11

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

It is the same as bug 785734 (except for a different function) so it has to be sec-critical as well.

Comment 12

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

Critsmash triage: Does this affect ESR 10 and other Firefox versions? How far back does this go?

Comment 13

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

It certainly affects every Firefox version since we shipped WebGL i.e. Firefox 4.

Comment 14

[Alex Keybl [:akeybl]]

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.

Comment 15

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

I would appear that I forgot to land this :-(

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

Comment 16

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

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:

Comment 17

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

Erm, approval request comment: trivial fix for critical sec bug.

Comment 18

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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'

Comment 21

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

Indeed, esr10 is significantly different there. Will make a patch.

Comment 22

[Matt Wobensmith [:mwobensmith][:matt:]]

Confirmed the crash on 2012-10-17 nightly using provided bug file.
Cannot reproduce on ESR 10.0.10.
Ubuntu 11.10

Comment 23

[Alex Keybl [:akeybl]]

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

Comment 24

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

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 :-)