Investigate whether proxies other than CCWs store a cross compartment pointer in their private slot

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jonco, Assigned: jonco)

Tracking

55 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
I didn't think this was supposed to happen but the assertion failures in bug 1365564 after the initial patch landed suggest that it does.
This seems scary. The odds of these sorts of things handling cross-compartment pointers correctly is extremely low.
Assignee

Comment 2

2 years ago
I couldn't figure out what was causing this.  This patch just adds a couple of assertions to catch this if it ever happens.
Assignee: nobody → jcoppeard
Attachment #8873415 - Flags: review?(sphink)
Comment on attachment 8873415 [details] [diff] [review]
bug1367815-cc-proxies

Review of attachment 8873415 [details] [diff] [review]:
-----------------------------------------------------------------

nice
Attachment #8873415 - Flags: review?(sphink) → review+
Comment on attachment 8873415 [details] [diff] [review]
bug1367815-cc-proxies

Review of attachment 8873415 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/ProxyObject.cpp
@@ +92,5 @@
>      values->init(proxy->numReservedSlots());
>  
>      proxy->data.handler = handler;
> +    if (IsCrossCompartmentWrapper(proxy))
> +        proxy->setCrossCompartmentPrivate(priv);

Would it be possible to assert this in setCrossCompartmentPrivate? IsCrossCompartmentWrapper will add a bunch of overhead in opt builds (it's not inlined).

Comment 5

2 years ago
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e5ac6fa7858
Add assertions to prevent proxies other than cross compartment wrappers from having cross compartment targets r=sfink

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9e5ac6fa7858
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.