1183882 - PushMessageData extends nsWrapperCache, but is not cycle collectable, nor has QI support for nsWrapperCache

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

Run the following in ServiceWorker:
var e = new PushEvent("test");
e.data.foo = e;


#0  0x00007fffed486d4b in nsWrapperCache::PreserveWrapper(nsISupports*) (this=0x7fffbabb09c8, aScriptObjectHolder=0x7fffbabb09c0)
    at /home/smaug/mozilla/hg/push-m-i/js/xpconnect/wrappers/../../../dom/base/nsWrapperCache.h:234
#1  0x00007fffee509ec3 in mozilla::dom::PreserveWrapperHelper<mozilla::dom::workers::PushMessageData, true>::PreserveWrapper(mozilla::dom::workers::PushMessageData*) (aObject=0x7fffbabb09c0)
    at ../../dist/include/mozilla/dom/BindingUtils.h:2639
#2  0x00007fffee509e95 in mozilla::dom::PreserveWrapper<mozilla::dom::workers::PushMessageData>(mozilla::dom::workers::PushMessageData*) (aObject=0x7fffbabb09c0)
    at ../../dist/include/mozilla/dom/BindingUtils.h:2646
#3  0x00007fffee4d93b5 in mozilla::dom::PushMessageDataBinding_workers::_addProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>) (cx=0x7fffc16b3480, obj=..., id=..., val=...)
    at ./PushMessageDataBinding.cpp:148
#4  0x00007ffff2169290 in js::CallJSAddPropertyOp(JSContext*, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>), JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>) (cx=0x7fffc16b3480, op=0x7fffee4d9350 <mozilla::dom::PushMessageDataBinding_workers::_addProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>)>, obj=..., id=..., v=...)
    at /home/smaug/mozilla/hg/push-m-i/js/src/jscntxtinlines.h:330
#5  0x00007ffff2112e22 in CallAddPropertyHook(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, JS::Handle<js::Shape*>, JS::Handle<JS::Value>) (cx=0x7fffc16b3480, obj=..., shape=..., value=...)
    at /home/smaug/mozilla/hg/push-m-i/js/src/vm/NativeObject.cpp:995
#6  0x00007ffff20c752f in AddOrChangeProperty(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, JS::Handle<JSPropertyDescriptor>) (cx=0x7fffc16b3480, obj=..., id=..., desc=...)
    at /home/smaug/mozilla/hg/push-m-i/js/src/vm/NativeObject.cpp:1177



Simple ServiceWorkerConsole https://github.com/smaug----/serviceworkerconsole, but don't know how to actually serve that as
https + html page. github pages don't seem to work for me.
but running that as localhost works.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

(note, I'm adding assertions to catch this kind of errors)

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8633791 [details] [diff] [review]
patch


[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Rather easily, I'd say

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Commit message could be
"Bug 1183882, properly implement wrapper caching on PushMessageData, r=nsm"

Which older supported branches are affected by this flaw?
I was told this is exposed in Aurora/Nightly

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

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
The patch should work with aurora too.

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

Comment 4

[(no longer active)]

poiru: it would be interesting to see if your CC analysis stuff would have caught this...

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

The assertions from Bug 1183604 did catch this.
I'll land that patch once the issues it found have been landed.
Luckily no issues exposed to web in release builds.

Comment 6

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

Comment on attachment 8633791 [details] [diff] [review]
patch

Approvals given for landing.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/71541c64c605

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/71541c64c605

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/837a1a106b42