Closed Bug 1210570 Opened 4 years ago Closed 4 years ago

|location + ""| is now spoofable

Categories

(Core :: XPConnect, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox43 --- unaffected
firefox44 + fixed

People

(Reporter: bholley, Assigned: jorendorff)

References

Details

(Keywords: regression, sec-high)

Attachments

(2 files, 1 obsolete file)

This is a regression from bug 1054756. I noticed some changes going by, and jorendorff confirmed. I'll attach a testcase.
Attached patch Tests. v1Splinter Review
Flags: needinfo?(jorendorff)
Are we still keeping notes in https://etherpad-mozilla.org/html5-cross-origin-objects what the specification eventually needs to say? I've been somewhat occupied with other work, but I do mean to get that in due course. And since I haven't been super involved it would be great if we kept that as an up-to-date summary.
> Are we still keeping notes in https://etherpad-mozilla.org/html5-cross-origin-objects
> what the specification eventually needs to say?

Note that this bug is about the _same_ origin case.
I see, so specification-wise, the problem here is that IDL hasn't been updated with the latest ECMAScript lingo. It does seem in the spirit of [Unforgeable] that this should not work. (Although [Unforgeable] at times saying it only applies to members does not make that very clear.)
It's not a matter of lingo either.  It's just that IDL defined [Unforgeable] in such a way that it nerfed all the ways you could mess with the object, then ES added new ways of messing with the object.  We just need to nerf those too, and audit any future such additions to the language before implementing them.  And perhaps write more forward-looking tests for possible future ways objects could be messed with, but that's hard...
Group: core-security → dom-core-security
I'm going to mark this high. Feel free to adjust as needed.
Keywords: sec-high
Attachment #8670918 - Flags: review?(bzbarsky)
Comment on attachment 8670918 [details] [diff] [review]
Ensure that ToPrimitive(location) is not spoofable

r=me, but please make sure there's a webidl issue on file.
Attachment #8670918 - Flags: review?(bzbarsky) → review+
Filed <https://www.w3.org/Bugs/Public/show_bug.cgi?id=29183>.
Flags: needinfo?(jorendorff)
Allen raises a good question in that w3.org issue: can't we just have @@toPrimitive be a value property with the value undefined?
Flags: needinfo?(jorendorff)
(In reply to Boris Zbarsky [:bz] from comment #10)
> Allen raises a good question in that w3.org issue: can't we just have
> @@toPrimitive be a value property with the value undefined?

Implementing.
Flags: needinfo?(jorendorff)
Bah, defining a data property isn't something this API really supports. I wonder if I should add support for it or route around...
We can handle this the same way we handle toJSON on Location for now, via the explicit bit in InitUnforgeablePropertiesOnHolder in Codegen.py.
Attachment #8670918 - Attachment is obsolete: true
Comment on attachment 8672703 [details] [diff] [review]
Ensure that ToPrimitive(location) is not spoofable

r=me, but this pattern:

  SYMBOL_TO_JSID(JS::GetWellKnownSymbol(aCx, JS::SymbolCode::toPrimitive))

is pretty annoying, though used in various places now.  It sure would be nice if we had a way to do WELL_KNOWN_SYMBOL_ID(aCx, toPrimitive) and have it work (all caps because it'd have to be a macro).
Attachment #8672703 - Flags: review?(bzbarsky) → review+
Blocks: 1214375
https://hg.mozilla.org/mozilla-central/rev/48d1f07e8d39
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Group: dom-core-security → core-security-release
Group: core-security-release
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.