Closed Bug 1072847 Opened 10 years ago Closed 10 years ago

Crash [@ _moz_cairo_surface_destroy] with long DOM Notification, only on Windows


(Core :: Graphics, defect)

Windows 7
Not set



Tracking Status
firefox33 --- wontfix
firefox34 --- verified
firefox35 --- verified
firefox36 + verified
firefox-esr31 34+ verified
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.2 --- unaffected


(Reporter: jruderman, Assigned: jrmuizel)



(4 keywords, Whiteboard: [adv-main34+][adv-esr31.3+])


(3 files)

Attached file testcase
      No description provided.
Attached file stack
Crash at poisoned memory, I'm going to call this critical-uaf based on my first glance.
Is this crashing the nightly, or does it need to be asan build or some such?  I don't crash on my Windows 7 with 2014-09-25 nightly with this about:support graphics section:

Adapter Description	NVIDIA Quadro 600
Adapter Drivers	nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM	1024
ClearType Parameters	Gamma: 2200 Pixel Structure: R ClearType Level: 100 Enhanced Contrast: 50
Device ID	0x0df8
Direct2D Enabled	true
DirectWrite Enabled	true (6.2.9200.16571)
Driver Date	5-19-2014
Driver Version
GPU #2 Active	false
GPU Accelerated Windows	1/1 Direct3D 11 (OMTC)
Subsys ID	083510de
Vendor ID	0x10de
WebGL Renderer	Google Inc. -- ANGLE (NVIDIA Quadro 600 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote	true
AzureCanvasBackend	direct2d
AzureContentBackend	direct2d
AzureFallbackCanvasBackend	cairo
AzureSkiaAccelerated	0
Crashed for me with Win7SP1 and 9-25.

Adapter Description: NVIDIA Quadro 600
Adapter Drivers: nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Adapter RAM: 1024
ClearType Parameters: D [ Gamma: 1800 Pixel Structure: R ClearType Level: 100 Enhanced Contrast: 50 ] D [ Gamma: 1800 Pixel Structure: R ClearType Level: 100 Enhanced Contrast: 50 ]
Device ID: 0x0df8
Direct2D Enabled: true
DirectWrite Enabled: true (6.2.9200.16571)
Driver Date: 12-29-2012
Driver Version:
GPU #2 Active: false
GPU Accelerated Windows: 1/1 Direct3D 11 (OMTC)
Subsys ID: 083510de
Vendor ID: 0x10de
WebGL Renderer: Google Inc. -- ANGLE (NVIDIA Quadro 600 Direct3D9Ex vs_3_0 ps_3_0)
windowLayerManagerRemote: true
AzureCanvasBackend: direct2d
AzureContentBackend: direct2d
AzureFallbackCanvasBackend: cairo
AzureSkiaAccelerated: 0
Aha, all the app notes say: Attempt to create DrawTarget for invalid surface

So these surfaces were already previously destroyed by InitAlreadyReferenced from bug 1069584.
Flags: needinfo?(bas)
We probably shouldn't let this sit too long...
There was a large spike in these crashes with nightly build 20141011030203. In builds since then, this is the #2 crash on Win32, #1 on Win64.
Unsure this is graphics, but since this is urgent, and dealing with the time difference, Jeff will take a look as well.
Flags: needinfo?(jmuizelaar)
It looks like this is caused by trying to create D3D11 compositor for a very wide window and things going bad from there.
Flags: needinfo?(jmuizelaar)
Attachment #8504817 - Flags: review?(bgirard)
Attachment #8504817 - Flags: review?(bgirard) → review+
Comment on attachment 8504817 [details] [diff] [review]
Initialize mSurface

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

I don't think very easily. This is basically using an initialized member. There are no vtables on the member so getting execution seems hard.

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


Which older supported branches are affected by this flaw?

Probably all of them.

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

The backports would be easy.

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

It should not cause regressions and shouldn't need very much testing.
Attachment #8504817 - Flags: sec-approval?
Comment on attachment 8504817 [details] [diff] [review]
Initialize mSurface

Please check in on Oct 28 for trunk. Sec-approval+.

We'll want this on Aurora, Beta, and ESR31 at that point as well. Can you prepare and nominate patches?
Attachment #8504817 - Flags: sec-approval? → sec-approval+
Assignee: nobody → jmuizelaar
Flags: needinfo?(bas)
Comment on attachment 8504817 [details] [diff] [review]
Initialize mSurface

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Use after free, security hole.
[Describe test coverage new/current, TBPL]:
[Risks and why]: None, we're initializing uninitialized variable.
[String/UUID change made/needed]: None.

Same patch applies cleanly to Aurora and Beta.
Attachment #8504817 - Flags: approval-mozilla-beta?
Attachment #8504817 - Flags: approval-mozilla-aurora?
Attachment #8504817 - Flags: approval-mozilla-beta?
Attachment #8504817 - Flags: approval-mozilla-beta+
Attachment #8504817 - Flags: approval-mozilla-aurora?
Attachment #8504817 - Flags: approval-mozilla-aurora+
Can't land until the 28th per comment 13. Also, needs an esr31 nomination still.
Keywords: checkin-needed
My bad - I read comment 13 as "make sure this lands before or on Oct 28th", but I guess it means "please do not land before Oct 28th".
It's unfortunate that the checkin has had to wait so long. Meanwhile this continues to be a topcrash on nightly and aurora. It's not clear to me why waiting is better in this instance.
It is almost the 28th now, so it doesn't matter much, but in the future if there's some stability concern you should mention that in the approval request, or followup with Al Billings or Dan Veditz to see if something could be landed sooner.  There wasn't anything in this bug prior to your comment that indicated this was a top crash.
The topcrash is being tracked as the dependent bug 1071079. You're right that it's not obvious to someone just looking at this bug. I'll do better about calling attention to this kind of thing in the future.
Comment on attachment 8504817 [details] [diff] [review]
Initialize mSurface

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: sec-critical
User impact if declined: sec-critical
Fix Landed on Version: not landed yet
Risk to taking this patch (and alternatives if risky): low, simple patch
String or UUID changes made by this patch: nope
Attachment #8504817 - Flags: approval-mozilla-esr31?
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Attachment #8504817 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31+
Whiteboard: [adv-main34+][adv-esr31.3+]
Reproduced the original crash several times on my Win 7 SP1 x64 (VM) and Win 8.1 x64 using the following build:

OS's used:

- Windows 8.1 x64
- Windows 7 SP1 x64 (VM)

Went through verification using the following builds:

Went through the following test cases for each OS/Build listed above:

- opened the poc in several different windows/tabs
- opened the poc in several different private windows/tabs
- opened the poc in several e10s windows/tabs
Group: core-security
You need to log in before you can comment on or make changes to this bug.