Expose caller principal to JS-implemented webidl

RESOLVED FIXED in mozilla30

Status

()

defect
RESOLVED FIXED
5 years ago
2 months ago

People

(Reporter: mt, Assigned: bholley)

Tracking

(Blocks 2 bugs)

unspecified
mozilla30
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(7 attachments, 2 obsolete attachments)

5.00 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
2.84 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
2.49 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.10 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
10.44 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.15 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
4.69 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review
Reporter

Description

5 years ago
JS-implemented webidl is unable to accurately (i.e., securely) determine the principal of the content object that invoked it.  Using window.document.nodePrincipal exposes the JS to the chance that navigation could occur, causing this to be spoofed.

In C++, this would involve calling GetSubjectPrincipal().
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8370985 - Attachment is obsolete: true
Assignee

Comment 3

5 years ago
Taking over from here.
Assignee: bzbarsky → bobbyholley
Assignee

Updated

5 years ago
Blocks: SH-bindings
Assignee

Comment 4

5 years ago
I've got all the code done for this - I just need to write tests.
Assignee

Comment 6

5 years ago
Still sorting out some weirdness with the build changes for the tests for bug 923904, but we can start getting this reviewed.
Assignee

Comment 8

5 years ago
This patch, and those following, are part of an epic quest to make this API
work properly despite the fact that we don't yet have GetEntryGlobal. See
the comment a few patches forward.
Attachment #8374948 - Flags: review?(bzbarsky)
Assignee

Comment 9

5 years ago
This will let us ask whether the AutoCxPusher is stack-top.
Attachment #8374949 - Flags: review?(bzbarsky)
Assignee

Comment 10

5 years ago
This will allow us to downcast from a stack entry to an AutoEntryGlobal, and
thereby get at the AutoCxPusher.
Attachment #8374950 - Flags: review?(bzbarsky)
Assignee

Comment 12

5 years ago
Attachment #8371028 - Attachment is obsolete: true
Attachment #8374952 - Flags: review?(bzbarsky)
Assignee

Comment 13

5 years ago
Intentionally not flagging these for review until we get the build issues
sorted.
Assignee

Comment 14

5 years ago
Hopefully sorted out the packaging issues: https://tbpl.mozilla.org/?tree=Try&rev=9480abb18974
Assignee

Comment 15

5 years ago
Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

All green now. Flagging bz.
Attachment #8374954 - Flags: review?(bzbarsky)
Comment on attachment 8374947 [details] [diff] [review]
Part 1 - Add accessors to the script settings stack entries themselves, not just the globals. v1

r=me
Attachment #8374947 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374948 [details] [diff] [review]
Part 2 - Add an API to determine if a given AutoCxPusher corresponds to the stack-top cx push. v1

r=me
Attachment #8374948 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374949 [details] [diff] [review]
Part 3 - Use an AutoCxPusher directly in Auto{Entry,Incumbent}Global. v1

r=me
Attachment #8374949 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374950 [details] [diff] [review]
Part 4 - Make Auto{Entry,Incumbent}Global inherit ScriptSettingsStackEntry. v1

Do we want to mark these as stack classes while we're here?
Attachment #8374950 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374951 [details] [diff] [review]
Part 5 - Implement GetCallerPrincipalOverride. v1

How cheap is GetSubjectPrincipal?  We're going to do it on every JS-implemented WebIDL call/get/set, right?

>+            callSetup += str(bool(isJSImplementedDescriptor(self.descriptorProvider))).lower()

  callSetup += toStringBool(isJSImplementedDescriptor(self.descriptorProvider))

r=me modulo that performance concern.
Attachment #8374951 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374952 [details] [diff] [review]
Part 6 - Implement Cu.getWebIDLCallerPrincipal. v1

r=me
Attachment #8374952 - Flags: review?(bzbarsky) → review+
Comment on attachment 8374954 [details] [diff] [review]
Part 7 - Tests. v1

r=me
Attachment #8374954 - Flags: review?(bzbarsky) → review+
Assignee

Comment 23

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #19)
> Do we want to mark these as stack classes while we're here?

We could, but we may need to undo that again with bug 971673. I'd rather sort that out first.

(In reply to Boris Zbarsky [:bz] from comment #20)
> How cheap is GetSubjectPrincipal?  We're going to do it on every
> JS-implemented WebIDL call/get/set, right?

Pretty cheap. It boils down to:

nsJSPrincipals::get(GetCompartmentPrincipals(GetContextCompartment(XPCJSRuntime:Get()->GetJSContextStack()->Peek())))

So basically just pointer chasing. We do this several times for every wrap() call, for comparison.
Yes, and wrap() is slow....  I guess we're doing a bunch of it anyway here.  :(
Reporter

Updated

5 years ago
Blocks: 994935
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.