Closed
Bug 854480
Opened 12 years ago
Closed 12 years ago
Remove usage of old-style security wrapper unwrapping
Categories
(Core :: XPConnect, defect)
Tracking
()
RESOLVED
FIXED
mozilla23
People
(Reporter: bholley, Assigned: bholley)
References
Details
Attachments
(7 files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mrbkap
:
review+
|
Details | Diff | Splinter Review |
Now that GWNOJO is gone, I think we can get rid of XPCWrapper::Unwrap and friends and replace it with usage of js::UnwrapObjectChecked. This better reflects the modern security model and should generally be good defense-in-depth.
The main difference at present is that js::UnwrapObjectChecked refuses to unwrap COWs, whereas XPCWrapper::Unwrap will unwrap them. I think I have this covered by a special check in XPCConvert, but I might need to temporarily sprinkle that magic in a few other places as well.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
Attachment #731009 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #731010 -
Flags: review?(mrbkap)
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #731011 -
Flags: review?(mrbkap)
Assignee | ||
Comment 5•12 years ago
|
||
I don't claim to really understand why we can end up with a security wrapper
here, but this change should be equivalent.
Attachment #731012 -
Flags: review?(mrbkap)
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #731013 -
Flags: review?(mrbkap)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #731014 -
Flags: review?(mrbkap)
Assignee | ||
Comment 8•12 years ago
|
||
Attachment #731015 -
Flags: review?(mrbkap)
Assignee | ||
Comment 9•12 years ago
|
||
(I've fixed the two try oranges in these patches btw)
Comment 10•12 years ago
|
||
Comment on attachment 731009 [details] [diff] [review]
Part 1 - Remove old-style unwrapping from dom/bindings bindings. v1
> +++ b/dom/bindings/BindingUtils.h
>+ obj = js::UnwrapObjectChecked(obj, /* stopAtOuter = */ false);
This no longer has the SOW issues we used to have?
>+ // ObjectClassIs for this stuff and we can just kill this code.
Do please file a bug on that?
r=me
Attachment #731009 -
Flags: review?(bzbarsky) → review+
Updated•12 years ago
|
Attachment #731010 -
Flags: review?(mrbkap) → review+
Updated•12 years ago
|
Attachment #731011 -
Flags: review?(mrbkap) → review+
Comment 11•12 years ago
|
||
Comment on attachment 731012 [details] [diff] [review]
Part 4 - Remove old-style unwrapping from XPCWrappedJSClass. v1
I don't remember why I added that unwrap call there anymore... I made the change in the bug that added SOWs, and that stuff has all changed enough by now that we might be able to remove it.
Attachment #731012 -
Flags: review?(mrbkap) → review+
Updated•12 years ago
|
Attachment #731013 -
Flags: review?(mrbkap) → review+
Updated•12 years ago
|
Attachment #731014 -
Flags: review?(mrbkap) → review+
Updated•12 years ago
|
Attachment #731015 -
Flags: review?(mrbkap) → review+
Assignee | ||
Comment 12•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #10)
> This no longer has the SOW issues we used to have?
UnwrapObjectChecked is now dynamic for SOWs (though only when XBL scopes are disabled).
> >+ // ObjectClassIs for this stuff and we can just kill this code.
>
> Do please file a bug on that?
Filed bug 856833.
Assignee | ||
Comment 13•12 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/623c8b11f31f
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/b8a91b194ba6
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/544c73301d31
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/83ae75a544fb
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/18257899ab4b
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a6653dadb7f8
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/64771564b5bc
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/623c8b11f31f
https://hg.mozilla.org/mozilla-central/rev/b8a91b194ba6
https://hg.mozilla.org/mozilla-central/rev/544c73301d31
https://hg.mozilla.org/mozilla-central/rev/83ae75a544fb
https://hg.mozilla.org/mozilla-central/rev/18257899ab4b
https://hg.mozilla.org/mozilla-central/rev/a6653dadb7f8
https://hg.mozilla.org/mozilla-central/rev/64771564b5bc
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in
before you can comment on or make changes to this bug.
Description
•