function.caller returns null when called from different iframe

VERIFIED FIXED in Firefox 17

Status

()

defect
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: mahks1, Assigned: bholley)

Tracking

({regression})

15 Branch
mozilla19
x86
Windows 7
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox16+ wontfix, firefox17+ verified, firefox18+ verified)

Details

(Whiteboard: [js:p1:fx19])

Attachments

(2 attachments)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

upgraded to FF 15.0.1


Actual results:

Javascript failed on reference to function.caller.name


Expected results:

returned function name
under FF 14 same code returned function name,
under FF 14 returns NULL
should read 
under FF 15 returns NULL
Please provide a testcase and steps to reproduce.
Status: UNCONFIRMED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
It seems there is something that changed.
function.caller still works when the calling function is in same iframe,
but it is NULL when called from another iframe.

This has changed from v14 & previous, you used to get the caller.name even when called from another iframe.

Test case : call the following from a function in a different iframe

function test(){
    console.debug('Test caller.name '+test.caller.name)
}

This will return the caller.name in v14 but test.caller=NULL in v15
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Summary: function.caller returns null even when not called from top → function.caller returns null when called from different iframe
Possible to have an HTML testcase?
Assignee: nobody → general
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
Please try to reproduce the issue using the latest FF 18 Nightly ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-central/
This is a cpg regression.  Quoting Waldo:

<Waldo> bz: hmm; /* Censor the caller if it is from another compartment. */
<Waldo> bz: that made sense pre-cpg
<Waldo> bz: ...now not so much
<Waldo> I guess you want some sort of origin/principals check
* Waldo wonders what that even looks like in pure JS code these days
Blocks: cpg
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
(In reply to Boris Zbarsky (:bz) from comment #8)
> This is a cpg regression.  Quoting Waldo:
> 
> <Waldo> bz: hmm; /* Censor the caller if it is from another compartment. */
> <Waldo> bz: that made sense pre-cpg
> <Waldo> bz: ...now not so much
> <Waldo> I guess you want some sort of origin/principals check
> * Waldo wonders what that even looks like in pure JS code these days

We really want to do some kind of subsumes check on the origins. But luke ripped that callback out. ;-)

We could try to do a PUNCTURE on the wrapper, see if it throws, and clear the exception if it does. But really, I think we need some better API to do this. Some kind of callback that tells us whether the wrapper is allowed to be punctured (which is roughly equivalent to asking whether the compartment of the wrapper subsumes the compartment of the wrappee).

Luke, what do you think?
(In reply to Boris Zbarsky (:bz) from comment #8)
> This is a cpg regression.  Quoting Waldo:

I'm not sure the case can be made that this is a critical user regression given the fact that this was first reported weeks into the cycle. Can somebody help me understand the user impact that we're worried about here? This is likely a wontfix for FF16 at this point.
(In reply to Bobby Holley (:bholley) from comment #10)
> We could try to do a PUNCTURE on the wrapper, see if it throws, and clear
> the exception if it does. 

Can we just call "wrap"?  I forget whether it throws or returns some sort of opaque wrapper in the case of different-origin.

Failing that, a "subsumes" predicate on compartment makes sense.
(In reply to Luke Wagner [:luke] from comment #12)
> (In reply to Bobby Holley (:bholley) from comment #10)
> > We could try to do a PUNCTURE on the wrapper, see if it throws, and clear
> > the exception if it does. 
> 
> Can we just call "wrap"?  I forget whether it throws or returns some sort of
> opaque wrapper in the case of different-origin.

It will be an opaque wrapper. Whether that's OK or not depends entirely on the purpose of the check. Waldo, can you elaborate on whether such an opaque wrapper would be ok?
> Can somebody help me understand the user impact that we're worried about here?

Some set of sites being broken.  What that set is, I can't tell you.  :(
(In reply to Bobby Holley (:bholley) from comment #13)
> It will be an opaque wrapper. Whether that's OK or not depends entirely on
> the purpose of the check. Waldo, can you elaborate on whether such an opaque
> wrapper would be ok?

Well, the previous code returned null for the cross-compartment (== cross-origin) case.  This would be returning an object instead, one the user couldn't use except in very limited ways.  That incompatibility probably means an opaque wrapper wouldn't be enough.  But it's impossible to say for sure, the web being what it is.
(In reply to Boris Zbarsky (:bz) from comment #14)
> > Can somebody help me understand the user impact that we're worried about here?
> 
> Some set of sites being broken.  What that set is, I can't tell you.  :(

We'll track for FF17 and on, but will wontfix for FF16 given where the investigation is and the fact that peoples' heads didn't explode when FF15 was released.
Whiteboard: [js:p1:fx19]
So basically, I think we need to modify the PUNCTURE infrastructure a little bit. As background, PUNCTURE is a way to ask a wrapper whether you can safely unwrap it from a security perspective.

I originally implemented it as an |action| for the |enter()| policy enforcement trap. This worked ok, but had the nasty side effect that you couldn't ask the question without PUNCTURE throwing if you got it wrong. That led to nasty exception-munging code like this:

http://hg.mozilla.org/mozilla-central/file/c9a8f55d8541/dom/plugins/base/nsJSNPRuntime.cpp#l469

So I think we need to rework this a little bit. I'd like to move the PUNCTURE logic to a simple, separate predicate on the proxy handler itself, which we could then query (without necessarily throwing).

So, we could implement such a predicate manually. But I've been wondering if it might align well with SecurityWrapper, which we have yet to use for anything substantial. The big question here is whether SecurityWrapper is intended to be used for all security wrappers, or just ones where the wrapper does not subsume the wrappee (I'm thinking in particular here of Xrays, where we have a special security policy to protect chrome from content, but chrome should be able to unwrap the content wrapper if it wants). Luke, can you weigh in on that? If SecurityWrapper is indeed intended only for the !Subsumes(wrapper,wrappee) case, then we can just implement CanSafelyUnwrap by checking whether we're a SecurityWrapper or not.
Trying out this fancy new bugzilla flag.
Flags: needinfo?(luke)
[Nice flag, shows up better on radar.]

SecurityWrapper was meant to be a ProxyHandler that defaulted to "no" instead of "yes".  We later had to nerf it because of document.domain sadness, but, with cpg, we could probably turn it back into a "default to no" version of Wrapper.  Other than this vague motivation, it has no more specific semantics; feel free to mold it as you see fit; you certainly understand the wrapping/privilege situation better than me.
Flags: needinfo?(luke) needinfo?(luke) → needinfo+
Depends on: 800915
Bobby, this is now depending on a bug that is not tracked for 17 - what is your estimate of being able to get a speculative fix here in the next two weeks (the time we have for such things on FF 17 Beta)?
Also - assigning to Bobby as we need to have people assigned to 17 tracked bugs at this stage.
Assignee: general → bobbyholley+bmo
Bug 800915 isn't going to be suitable for landing on beta, so let's just fix this bug the ugly way for now.

https://tbpl.mozilla.org/?tree=Try&rev=add4f9d09ff3
Comment on attachment 672248 [details] [diff] [review]
Only censor function.caller for non-same-origin calls. v1

Makes sense.

Nit: end of comment should be:

 * NB - This will get much nicer with bug 800915
 */
Attachment #672248 - Flags: review?(luke) → review+
Comment on attachment 672248 [details] [diff] [review]
Only censor function.caller for non-same-origin calls. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): CPG
User impact if declined: potential web breakage.
Testing completed (on m-c, etc.): Just pushed to m-i. Please sit on this approval request until the m-c bake time has happened. I'm a bit overloaded right now and I don't want to risk forgetting to nom this.
Risk to taking this patch (and alternatives if risky): Low risk 
String or UUID changes made by this patch: None
Attachment #672248 - Flags: approval-mozilla-beta?
Attachment #672248 - Flags: approval-mozilla-aurora?
Triage drive by - sitting on approval until this has baked on m-c a bit.
https://hg.mozilla.org/mozilla-central/rev/1eb12b546fad
Status: NEW → RESOLVED
Closed: 7 years ago7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment on attachment 672248 [details] [diff] [review]
Only censor function.caller for non-same-origin calls. v1

Feel free to let this sit on m-c a little longer if you need to, but let's make sure this gets landed by Monday Oct 22nd so it's in our next Beta.
Attachment #672248 - Flags: approval-mozilla-beta?
Attachment #672248 - Flags: approval-mozilla-beta+
Attachment #672248 - Flags: approval-mozilla-aurora?
Attachment #672248 - Flags: approval-mozilla-aurora+
Reproducible on FF 15.0.1.
Verified fixed on FF 17b5, FF 18.0a2 (2012-11-12) on Win 7.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.