Closed Bug 1029248 Opened 5 years ago Closed 5 years ago

Allow CPOWs to be converted to native interfaces

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(2 files, 2 obsolete files)

The purpose of this bug is to allow CPOWs to masquerade as interfaces like HTMLDocument, which normally don't allow scripted implementations.
Attached patch cpow-wrapping (obsolete) — Splinter Review
This patch adds a new IsWrappedCPOW that unwraps an object before testing if it's a CPOW. There were a few existing places where we did this, and I found more where we were missing the unwrapping. I also think CheckedUnwrapped is the way to go. We absolutely shouldn't be exposing CPOWs to content.
Attachment #8444835 - Flags: review?(mrbkap)
Attached patch cpow-scriptable (obsolete) — Splinter Review
If we need to pass a CPOW to a function over XPIDL, we may need to QI it to some non-scriptable interface like HTMLDocument. This patch bypasses the checks that normally would forbid that.

I think the main danger here is that C++ code will assume that it can cast an nsIHTMLDocument or whatever to something concrete. I'm going to try to figure out a way that we can prevent passing CPOWs to C++ code, possibly with some exceptions whitelisted. However, I'd like to take this now so that we can make progress on add-on compatibility.
Attachment #8444843 - Flags: review?(mrbkap)
Attached patch cpow-wrapping v2Splinter Review
I probably should have tried to compile these before posting them.
Attachment #8444835 - Attachment is obsolete: true
Attachment #8444835 - Flags: review?(mrbkap)
Attachment #8445587 - Flags: review?(mrbkap)
Attachment #8444843 - Attachment is obsolete: true
Attachment #8444843 - Flags: review?(mrbkap)
Attachment #8445588 - Flags: review?(mrbkap)
Attachment #8445587 - Flags: review?(mrbkap) → review+
Comment on attachment 8445588 [details] [diff] [review]
cpow-scriptable v2

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

This is OK because we also have builtinclass, right?
Attachment #8445588 - Flags: review?(mrbkap) → review+
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4907115e02e3
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4a19dcbc7e7a

(In reply to Blake Kaplan (:mrbkap) from comment #5)
> This is OK because we also have builtinclass, right?

Yes, that should offer some protection.
https://hg.mozilla.org/mozilla-central/rev/f8be8f1a923b
https://hg.mozilla.org/mozilla-central/rev/c7df28f9f545
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.