Closed Bug 1201684 Opened 4 years ago Closed 4 years ago

Creating a compositor via GetLayerManager runs script at unsafe times

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: bzbarsky, Assigned: dvander)

References

(Blocks 1 open bug)

Details

(Whiteboard: gfx-noted)

Attachments

(1 file)

Stack:

#0  mozilla::dom::AutoEntryScript::AutoEntryScript (this=0x7fff5fbfb878, aGlobalObject=0x120a90a00, aReason=0x108aa913d "XPCWrappedJS method call", aIsMainThread=true, aCx=0x0) at ScriptSettings.cpp:544
#1  0x0000000103988f7c in mozilla::dom::AutoEntryScript::AutoEntryScript (this=0x7fff5fbfb878, aGlobalObject=0x120a90a00, aReason=0x108aa913d "XPCWrappedJS method call", aIsMainThread=true, aCx=0x0) at ScriptSettings.cpp:549
#2  0x0000000102f3b000 in nsXPCWrappedJSClass::CallMethod (this=0x11fe27ce0, wrapper=0x12c8ca500, methodIndex=3, info_=0x117dbc978, nativeParams=0x7fff5fbfbac0) at XPCWrappedJSClass.cpp:964
#3  0x0000000102f3af05 in nsXPCWrappedJS::CallMethod (this=0x12c8ca500, methodIndex=3, info=0x117dbc978, params=0x7fff5fbfbac0) at XPCWrappedJS.cpp:525
#4  0x0000000101f61493 in PrepareAndDispatch (self=0x120c7a720, methodIndex=3, args=0x7fff5fbfbbe0, gpregs=0x7fff5fbfbb60, fpregs=0x7fff5fbfbb90) at xptcstubs_x86_64_darwin.cpp:122
#5  0x0000000101f5fe9b in SharedStub () at nsDebug.h:81
#6  0x0000000101e97779 in nsObserverList::NotifyObservers (this=0x1203fe038, aSubject=0x0, aTopic=0x108afb501 "compositor:created", someData=0x0) at nsObserverList.cpp:113
#7  0x0000000101e997f1 in nsObserverService::NotifyObservers (this=0x100410580, aSubject=0x0, aTopic=0x108afb501 "compositor:created", aSomeData=0x0) at nsObserverService.cpp:315
#8  0x000000010364e028 in gfxPlatform::NotifyCompositorCreated (this=0x120aa9a40, aBackend=mozilla::layers::LAYERS_OPENGL) at ../../../mozilla/gfx/thebes/gfxPlatform.cpp:2488
#9  0x0000000105b8b3f7 in nsBaseWidget::CreateCompositor (this=0x12a6a8800, aWidth=200, aHeight=200) at ../../mozilla/widget/nsBaseWidget.cpp:1148
#10 0x0000000105b8a277 in nsBaseWidget::CreateCompositor (this=0x12a6a8800) at ../../mozilla/widget/nsBaseWidget.cpp:866
#11 0x0000000105bec5ef in nsChildView::CreateCompositor (this=0x12a6a8800) at ../../../mozilla/widget/cocoa/nsChildView.mm:1930
#12 0x0000000105b8b4ff in nsBaseWidget::GetLayerManager (this=0x12a6a8800, aShadowManager=0x0, aBackendHint=mozilla::layers::LAYERS_NONE, aPersistence=LAYER_MANAGER_CURRENT, aAllowRetaining=0x7fff5fbfc0ef) at ../../mozilla/widget/nsBaseWidget.cpp:1167
#13 0x00000001037fecad in nsIWidget::GetLayerManager (this=0x12a6a8800, aAllowRetaining=0x7fff5fbfc0ef) at nsIWidget.h:1155
#14 0x000000010611532e in PresShell::Paint (this=0x120cf6000, aViewToPaint=0x12ab18b00, aDirtyRegion=@0x7fff5fbfc188, aFlags=1) at nsPresShell.cpp:6032

Pretty sure that Paint() does not expect script to run under it (e.g. it has no protection against "aViewToPaint" or "frame" dying a horrible death during that GetLayerManager call).
Now the good news is that this is running chrome script, not page script.  But this is assuming that all the observer implementations here will be very well-behaved and not do anything that can run script on any web page....
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryEnvironment.jsm#855 is the script.

Which is clearly safe, although it would be reasonable to delay this until the next event loop iteration as well. not s-s.

From bug 1175091
Blocks: 1175091
Group: core-security
Flags: needinfo?(dvander)
Flags: needinfo?(alessio.placitelli)
> Which is clearly safe,

Sure, but nothing stops extensions from listening for this observer notification...

If delaying it on a scriptrunner (doesn't even need to be a full event loop spin) is not a problem, that would certainly solve the problem I'm trying to solve here.
Whiteboard: gfx-noted
Attached patch fixSplinter Review
Assignee: nobody → dvander
Status: NEW → ASSIGNED
Flags: needinfo?(dvander)
Attachment #8658403 - Flags: review?(matt.woodrow)
Attachment #8658403 - Flags: review?(matt.woodrow) → review+
Flags: needinfo?(alessio.placitelli)
https://hg.mozilla.org/mozilla-central/rev/8d2881862098
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.