198660 - Frame doesn't load (regression on trunk) (Function.href access denied)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Olivier Cahagne]

build ID: 2003032108 on Linux and Win2k.
works fine with Mozilla 1.3 (release) on Linux & Win2k.

Steps to reproduce:
1. Load URL http://www.airfrance.fr/
2. Fill in the form on the left frame with mozilla / moz1lla
3. Click on "Accès mon Air France" just below form,
4. Notice the right main frame not being refreshed 

Mozilla 1.3 will load content on the main frame after having displayed the new
menu on the left frame where the form was displayed.

I noticed this on trunk (1.4a) 2 or 3 weeks ago, I cannot reduced the regression
window as builds are no more available (I'd need some late February builds, if
someone can test...)

Couldn't come with a reduced testcase, it involves frames.
This is not bug 113224, but a real regression. I don't think this is bug 192312
either as the steps above work on 1.3 branch.

Comment 1

[Boris Zbarsky [:bzbarsky]]

*** Bug 198480 has been marked as a duplicate of this bug. ***

Comment 2

[Boris Zbarsky [:bzbarsky]]

This broke between 2003-03-06-08 and 2003-03-07-08.

Comment 3

[Olivier Cahagne]

could it be a regression from bug 196130 ?

Comment 4

[Boris Zbarsky [:bzbarsky]]

I see no particularly relevant checkins on that day, but the site is trying to
access the location of a frame loaded from double6.airfrance.fr.  The full URL is:

http://double6.airfrance.fr/double6/FR/infolocale.nsf/(LookupPublishedWeb)/fr-1ACCU-HP_France?OpenDocument

With a minimal document as follows:

<FRAMESET ROWS='*'>
  <FRAME NAME='content'
SRC='http://double6.airfrance.fr/double6/FR/infolocale.nsf/(LookupPublishedWeb)/fr-1ACCU-HP_France?OpenDocument'>
</FRAMESET>

If I run javascript:alert(top.content.location) from the URL bar I get the
following error:

Error: uncaught exception: [Exception... "Could not convert JavaScript argument
arg 0 [nsIDOMWindowInternal.alert]"  nsresult: "0x80570009
(NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame ::
javascript:alert(top.content.location) :: <TOP_LEVEL> :: line 1"  data: no]

the typeof(top.content.location) is "object" and when I try to get its href I get:

Error: uncaught exception: Permission denied to get property Function.href

If I just load that URL directly instead of through a frame, I can access
window.location with no problems.

Comment 5

[Olivier Cahagne]

confirming the error in console when logging in using CVS 20030323 on Linux with
http://www.airfrance.fr/

JavaScript error:
 line 0: uncaught exception: Permission denied to get property Function.href

Comment 6

[HARUNAGA Hirotoshi]

Is this invalid ?

http://www.mozilla.org/projects/security/components/same-origin.html
The Same Origin Policy
  The same origin policy prevents document or script loaded from one
  origin from getting or setting properties of a document from a different
  origin.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yes, but why is it firing in this case?  This is why I cced mitch...  More to
the point, why was it not firing before?

Comment 8

[Boris Zbarsky [:bzbarsky]]

This is coming up a lot... does anyone know what's up?

Comment 9

[Johnny Stenback (:jst)]

I don't know what the deal really is here, it really does look like a same
origin problem to me too. We fail to access top.content.location here since
top.content comes from http://bv.airfrance.fr/cgi-bin/FR/frameset.jsp and top
(where the javascript: URL is executed) comes from
"http://double6.airfrance.fr/double6/FR/infolocale.nsf/(LookupPublishedWeb)/fr-1ACCU-HP_France?OpenDocument".
So different domains. There are no calls to set document.domain, so I don't know
how that site ever worked if it relies on communicating between frames (I didn't
have time to look at what the site does yet).

Comment 10

[Boris Zbarsky [:bzbarsky]]

OK, this was caused by the checkin for bug 92773 (tested by backing out that
patch locally).  Looking at all three pages (this one and the two in the blocked
bugs) they have frames loaded from one server trying to access frames loaded
from another server -- typically to set their location.  So it looks like we are
smacking them with same-origin policies of some sort....

Is .location being treated as a "user-set function" for some reason?  That would
explain the errors in the JS console, and would make sense in light of the
checkin comments for the bug 92773 patch.

In any case, there are two issues to be resolved here:

1)  Should we be blocking these accesses?
2)  If so, we need a useful error message -- the one we give right now is
    anything but.

Over to Mitch to make a call on #1.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Oh, and one more thing.  In bug 201337 we have two frames both in domain foo.com
and the frameset in domain bar.com and the frames are trying to communicate with
each other and getting security exceptions.... (and again, it's fixed if I
locally back out bug 92773).

Comment 12

[Boris Zbarsky [:bzbarsky]]

*** Bug 201927 has been marked as a duplicate of this bug. ***

Comment 13

[Asa Dotzler [:asa]]

mitch, what's the right thing to do here and can we make this any better than it
is for 1.4?

Comment 14

[Mitchell Stoltz (not reading bugmail)]

Brendan, any ideas on how to fix this regression from the fix for bug 92773?

Comment 15

[Vincent Deconinck]

As I understand it, this regression in 1.4 alpha (which blocks access to 3
identified sites - cfr bugs blocked by this one - among which the first French
airline company) is known to be caused by a fix.
I guess the reason to not back out this fix is that it involves a security issue
(hence the fact that I can't access 92773)
The problem is that keeping access to this bug in restricted mode only allows
very few developers to fix this. 
Are we going to ship 1.4 beta with a regression that's known for more than one
month ? Shouldn't this block 1.4 beta ?

Comment 16

[Mitchell Stoltz (not reading bugmail)]

vdeconinck, patches are welcome.

Comment 17

[Asa Dotzler [:asa]]

brendan asked me to assign this to him.

Comment 18

[Boris Zbarsky [:bzbarsky]]

> vdeconinck, patches are welcome.

Patches are impossible to produce without knowing what problem bug 92773 was
trying to address and why the code is done the way it is.  The fact that changes
nor relevant to the security fix were rolled in does not help -- perhaps the bug
describes which parts of the change are relevant to the security fix, but it's
not accessible.

Comment 19

[Brendan Eich [:brendan]]

Boris, why do you write "The fact that changes no[t] relevant to the security
fix were rolled in does not help"?  Do you have any evidence for that "fact"? 
It just so happens to be false.

mstoltz, how about opening bug 92773, or at least cc'ing bz on it.

Security fixes have unintended consequences -- news at 11.  I'm working on this,
it'll be fixed for 1.4final.

/be 

Comment 20

[Brendan Eich [:brendan]]

*** Bug 201337 has been marked as a duplicate of this bug. ***

Comment 21

[Brendan Eich [:brendan]]

The "Function" in "Function.href" is a stupid bug in the fix for bug 92773. 
That's easy to fix.  Working on the harder fix: maintain security but let native
getters and setters be called (they're responsible for doing their own security
checks).

/be

Comment 22

[Brendan Eich [:brendan]]

Attachment: fix attempt

The jsinterp.c change is ugly, but I don't see a better way.  JS relies on
native callbacks (functions, getters and setters) to do their own security
checking, and calls a checkAccess (object-class-specific) or checkObjectAccess
(runtime-wide) callback only for cases not covered by existing native
callbacks.

In fixing bug 92773, I used too broad a check for the getter/setter case.  We
want to allow native setters (such as the one for location.href) to be called
across origins, and we even let window.location's getter succeed across
origins. Only scripted getters and setters need protection to close 92773.

/be

Comment 23

[Brendan Eich [:brendan]]

Comment on attachment 122795 [details] [diff] [review]
fix attempt

Testing help here, and regression-proofing for bug 92773, craved.

/be

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 122795 [details] [diff] [review]
fix attempt

sr=jst

Comment 25

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 122795 [details] [diff] [review]
fix attempt

Looks good, r=mstoltz.

Comment 26

[Brendan Eich [:brendan]]

Comment on attachment 122795 [details] [diff] [review]
fix attempt

This should go in ASAP.

/be

Comment 27

[Phil Schwartau]

I ran the patch to jsinterp.c against the JS testsuite
on WinNT, and it caused no test regressions. 

cc'ing Jesse from bug 92773 -

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 122795 [details] [diff] [review]
fix attempt

a=asa (on behalf of drivers) for checkin to 1.4.

Comment 29

[Brendan Eich [:brendan]]

Fixed, thanks for the reviews.  Optimizing for the case where this tests well by
closing the bug -- reopen it if anyone finds something amiss.

/be

Comment 30

[WADA:World Anti-bad-Duping Agency]

I comfirmed that 2003050908-trunk/Win-Me resolved IBM Japan's same problem as
bug 198480 (dupe of this bug, Parent window<->Child window case).   

Comment 31

[(not reading, please use [e-mail] instead)]

this caused a regression.

#205236

Comment 32

[(not reading, please use [e-mail] instead)]

oops, maybe this wasn't the cause.  sorry for the false alarm.

Comment 33

[HARUNAGA Hirotoshi]

*** Bug 206261 has been marked as a duplicate of this bug. ***