38959 - watching JS variables from another window

Status: VERIFIED FIXED

(Core :: Security, defect, P3)

Description

[Mitchell Stoltz (not reading bugmail)]

Date: 
          Thu, 11 May 2000 16:20:17 +0300
   From: 
          Georgi Guninski <[e-mail]>
      To: 
          Mitchell Stoltz <[e-mail]>




Usually, Mozilla does not allow accessing JS variables from windows from
another host, so the following does NOT work and should not work:
---
a=window.open("http://site/setvar1"); 
setTimeout("alert(a.var1)",2000);
---
But this may be circumvented by using the watch() method immediately
after window.open().

The code is:
--------var1.html------------------------------------------------------
<SCRIPT>
a=window.open("http://site/setvar1.html");
a.watch("var1",function (id,ol,ne) {alert(id + " is set to: "+ne)} );
</SCRIPT>
----------------------------------------------------------------------

--------setvar1.html--------------------------------------------------
<SCRIPT>
var1="Just a variable";
</SCRIPT>
----------------------------------------------------------------------

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Reassigning to rogerl - I think the problem here is in the engine. Norris
checked in a fix to a similar problem, look at
http://bugzilla.mozilla.org/show_bug.cgi?id=28390 and the fix at
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/js/src&command=DIFF_FRAMESET&file=jsdbgapi.c&rev1=3.17&rev2=3.18&root=/cvsroot

Basically, we need to do a same-origin security check on the watch() function or
otherwise limit its use. As watch() belongs to Object.prototype, it's possible
this check should go in the engine somehow, although it could probably go in the
DOM as well. There's a testcase in the URL field above. Could you give me your
thoughts on where this security check should go?

Comment 2

[leger]

Putting on [nsbeta2+] radar for beta2 fix. 

Comment 3

[John Unruh]

Changing Qa to czhang.

Comment 4

[rogerl (gone)]

back atya

Comment 5

[Mitchell Stoltz (not reading bugmail)]

The problem here is that watch() gets called before window 'a' has loaded 
document "setvar1." Before the URL is loaded, the principal of the window is 
"about:blank," and setting watchpoints (as with any DOM access) is always allowed 
on about:blank. When setvar1 loads, the watchpoint persists. I'm not sure why 
this is so; as setvar1 loads, ClearObjectWatchpoint gets called in the js engine 
(though this may not be th watchpoint we're interested in). Simply replacing one 
document with another (using location=...) does clear watchpoints, see 
http://warp/u/mstoltz/bugs/38959a.html as an example.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Removing nsbeta2+. Not a beta2 stopper.

Comment 7

[Mitchell Stoltz (not reading bugmail)]

This should be fixed for beta3 as it's an exploit. For some reason, watchpoints 
don't seem to get cleared during the initial page reload (from about:blank to the 
actual URL requested). cc'ing rogerl and jst.

Comment 8

[Brendan Eich [:brendan]]

Trouble: I don't see JS_ClearWatchPointsForObject being called on the window 
object before each document load:

http://lxr.mozilla.org/mozilla/search?string=ClearWatchPointsForObject

It should be, or this bug and others will bite.  Mitch, where do you see a clear 
issuing from -- what's the full backtrace?

/be

Comment 9

[Mitchell Stoltz (not reading bugmail)]

I didn't save the full backtrace, but the calls to ClearWatchPointsForObject are 
not explicit. DOM calls JS_ClearScope which frees some references, then the DOM 
code explicitly calls GC, GC calls Finalize, and Finalize calls 
ClearWatchPointsForObject. Should this be made explicit? If another wondow is 
holding a reference to these objects, they won't be deleted by GC on document 
load, and the watchpoints should not be cleared in this case, rignt?

According to Vidur, clearing scope on an about:blank document breaks some pages. 
We couldn't think of any security problem in not clearing the scope, except for 
the watchpoints problem, so I wrote a patch to explicitly clear watchpoints in 
the case of about:blank, when ClearScope is not called. I'll post it here, can 
you review?

Comment 10

[Brendan Eich [:brendan]]

It's true that garbage objects, when finalized, clear any watchpoints on 
themselves.  But the problem here is that the window object lives across 
document loads, and so do its watchpoints, unless the DOM code explicitly calls 
JS_ClearWatchPointsForObject.  The old "classic" codebase did that -- see the 
lxr link I attached previously, look for files in lib/libmocha (the original 
"level 0 DOM" source).

/be

Comment 11

[Brendan Eich [:brendan]]

Just to be extra-clear (ahem, no pun): the way JS_ClearScope is called is fine 
and should not change, as vidur points out.  But there is a missing, different 
JS_ClearWatchPointsForObject call that needs to occur for the window object 
*before* every document load, including the real one that follows an implicit 
"load" of about:blank for a new window created but not yet loaded.

/be

Comment 12

[Mitchell Stoltz (not reading bugmail)]

Attachment: Patch - Brendan, can you review?

Comment 13

[Brendan Eich [:brendan]]

Attachment: proposed fix, with indentation Travis-ties undone

Comment 14

[Brendan Eich [:brendan]]

Attachment: diff -wu of fix for easier code reviewing

Comment 15

[Brendan Eich [:brendan]]

You don't want JS_ClearAllWatchPoints, that will nuke every watchpoint in the 
whole JS runtime.  We need only clear the window object's watchpoints, but we 
must do it whether or not there is a document already loaded (independent of 
mDocument being non-null).  Several people please buddy my change.

/be

Comment 16

[Mitchell Stoltz (not reading bugmail)]

Brendan,
   Your patch seems to fix the exploit. r=mstoltz.

Comment 17

[Brendan Eich [:brendan]]

Fix checked in, thanks for the help.

/be

Comment 18

[cathy zhang]

verified

Comment 19

[Mitchell Stoltz (not reading bugmail)]

Opening fixed security bugs to the public.