1028891 - (CVE-2014-1556) WebGL app crashes Firefox

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Patrick Cozzi]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.153 Safari/537.36

Steps to reproduce:

* Browse to http://cesiumjs.org/
* Click "Click to interact" to run the WebGL app


Actual results:

* Wait 10-30 seconds and Firefox crashes with a hard close and no noticeable new messages in the console.

This does not crash in Chrome or IE 11 and did not crash before Firefox 30.0

This is also reproducible in Firefox 32.0a2 (Aurora) and 33.0a1 (Nightly) on a variety of platforms (Windows, Linux, Mac) and GPUs.

This application loads tiled images for different parts of the globe as it spins, and creates WebGL textures for them.  The memory pressure or use of web workers for other processing may be the problem.

This is also reproducible by repeatedly zooming in and out in the simpler Cesium Viewer application (http://cesiumjs.org/Cesium/Build/Apps/CesiumViewer/index.html).  This is open-source (https://github.com/AnalyticalGraphicsInc/cesium).  We are happy to help debug and implement any workarounds.

Related discussion: https://groups.google.com/forum/#!topic/cesium-dev/6SLOxrZBqj4

Comment 1

[shunter]

I'm getting the same crash on Windows 7.  Crash is in mozilla::WebGLFramebufferAttachable::NotifyFBsStatusChanged() when deleting a WebGL texture.

Crash IDs: 

bp-5a90a1ce-291e-4a1a-8355-bd5252140623
bp-649bba9d-b563-4720-95b9-fc5aa2140623

Comment 2

[Alice0775 White]

Regression window(m-c)
Good:
https://hg.mozilla.org/mozilla-central/rev/092d63342910
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140314193520
Crashes
https://hg.mozilla.org/mozilla-central/rev/82c90c17fc95
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140314222109
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=092d63342910&tochange=82c90c17fc95

Regression window(m-i)
Good:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f9de0cc867b
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140314063947
Crashes
https://hg.mozilla.org/integration/mozilla-inbound/rev/af491832ff95
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 ID:20140314065948
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6f9de0cc867b&tochange=af491832ff95
Suspect: 
77b90f9bc6db	Dan Glastonbury — Bug 973625 - Implement WebGLFramebuffer completeness status caching. r=jgilbert

I can reproduce the crash on Beta31.0b2.
I can also reproduce the crash on Nightly33.0a1.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Dan

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Dan, does it ring a bell? thanks

Comment 5

[u480271]

I'll take a look.

Comment 6

[u480271]

I've tried the STR "This is also reproducible by repeatedly zooming in and out in the simpler Cesium Viewer application (http://cesiumjs.org/Cesium/Build/Apps/CesiumViewer/index.html)" in a debug build with assertions and I've crashed in the JS VM three times. Still trying to find a reliable repro for NotifyFBsStatusChanged().

Comment 7

[u480271]

Attachment: Detach textures/renderbuffers from framebuffer when deleting.

Framebuffer didn't detach texture/renderbuffer on deletion which caused use-after-free.

Comment 8

[Andrew McCreight [:mccr8]]

Please hide use-after-frees, particularly ones that affect release.

Comment 9

[u480271]

Try: https://tbpl.mozilla.org/?tree=Try&rev=2431c60731b8

Builds available for testing:
https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-2431c60731b8

Comment 10

[Andrew McCreight [:mccr8]]

This should get sec-approval+ before landing on inbound.  Just set the sec-approval? flag on the patch and fill out the form.

Comment 11

[u480271]

Comment on attachment 8445075 [details] [diff] [review]
Detach textures/renderbuffers from framebuffer when deleting.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

The patch suggests that this can be exploited by attaching a WebGL texture to a WebGL framebuffer and trigger the destruction of WebGL framebuffers to create an UAF but doesn't make it very obvious how.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Yes. Comments mention UAF. Check-in comment just says "Detach textures from framebuffer when deleting".


Which older supported branches are affected by this flaw?

FF30, FF31, FF32, FF33.

If not all supported branches, which bug introduced the flaw?

Bug 973625.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

The code in this patch should apply to all versions.

How likely is this patch to cause regressions; how much testing does it need?

The likelihood to create a regression is low.

Comment 12

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

Comment on attachment 8445075 [details] [diff] [review]
Detach textures/renderbuffers from framebuffer when deleting.

sec-approval+ for trunk. We should get this nominated and approved for Aurora and Beta as well.

Comment 13

[Sylvestre Ledru [:Sylvestre]]

If possible, I would like to have them in aurora and beta before beta 7 (Thursday).

Comment 14

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

Dan can we get this landed on trunk and get Aurora and Beta patches today?

Comment 15

[u480271]

Comment on attachment 8445075 [details] [diff] [review]
Detach textures/renderbuffers from framebuffer when deleting.

Approval Request Comment
[Feature/regressing bug #]: Bug 973625
[User impact if declined]: Ability to cause UAF with carefully constructed sequence of WebGL calls.
[Describe test coverage new/current, TBPL]: Tested on nightly and TPBL.
[Risks and why]: Code change risk is lower than leaving unpatched. The code existed but wasn't being called on object destruction.
[String/UUID change made/needed]:

Comment 16

[u480271]

Comment on attachment 8445075 [details] [diff] [review]
Detach textures/renderbuffers from framebuffer when deleting.

Approval Request Comment
[Feature/regressing bug #]: Bug 973625
[User impact if declined]: Ability to cause UAF with carefully constructed sequence of WebGL calls.
[Describe test coverage new/current, TBPL]: Tested on nightly and TPBL.
[Risks and why]: Code change risk is lower than leaving unpatched. The code existed but wasn't being called on object destruction.
[String/UUID change made/needed]:

Comment 17

[u480271]

I've checked locally and the patch applies cleanly to both Aurora and Beta.

Comment 18

[Lawrence Mandel [:lmandel] (use needinfo)]

Does this affect B2G? If so, we'll get it on 2.0 via the Aurora uplift. Should we take it on 1.4 (b2g30)?

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9aaf15b51ff2

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8445075 [details] [diff] [review]
Detach textures/renderbuffers from framebuffer when deleting.

Discussed with Ryan on IRC, even if it did not land on m-c before, approving preemptively to have it in beta 7 tonight. He will take care of the backout if that happens.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bd76b300b5a9
https://hg.mozilla.org/releases/mozilla-beta/rev/635e32a71297

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/0bb907070f61

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9aaf15b51ff2

Comment 25

[Patrick Cozzi]

I can confirm the fix with http://cesiumjs.org/

Thanks!  You guys rock.

Comment 26

[Kamil Jozwiak [:kjozwiak]]

Reproduced the original issue from comment #0 on OSX, Ubuntu and Windows using the following build:
* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-06-22-03-02-03-mozilla-central/

* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-07-07-03-02-02-mozilla-central/
** OSX 10.9.4 x64 [Passed]
** Ubuntu 13.10 x64 [Passed]
** Win 8.1 x64 [Passed]

* http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-07-07-00-40-06-mozilla-aurora/
** OSX 10.9.4 x64 [Passed]
** Ubuntu 13.10 x64 [Passed]
** Win 8.1 x64 [Passed]

* http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/31.0b7/
** OSX 10.9.4 x64 [Passed]
** Ubuntu 13.10 x64 [Passed]
** Win 8.1 x64 [Passed]


* ran the application for several minutes (3-8 minutes) without any crashes (previously crashed within 20-30s as mentioned in comment #0)
* used several of the controls without receiving a crash (zoom, pause, play, reversing through the timeline, changed the imagery, changing the view plane)
* increased/decreased the speed of the timeline without any crashes (left it at 604800x for a few minutes)
* ensured that closing the tab with the demo opened didn't produce any crashes

Comment 27

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

Follow-up: bug 1044668 changes the raw pointer  at the origin of this crash, into a WeakPtr - so that in the worst case, if a similar bug happens again around here, it will only be a null deref crash, not a security bug.