[ 
https://issues.apache.org/jira/browse/SHINDIG-416?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12610108#action_12610108
 ] 

John Hjelmstad commented on SHINDIG-416:
----------------------------------------

A rather lengthy and (IMHO) very interesting and useful thread quickly sprouted 
up regarding this technique among several people at Google. I asked if the 
thread could be moved to JIRA, and the involved participants agreed. As such, 
I'm copying it in below for further development, inspection, and/or insight. 
Apologies for the huge size... hopefully it's found useful.

Digested version, for those low on time (I hope this makes sense):
* Context: window.opener trick is being considered for gadgets.rpc use. Can it 
be made safe?
* What about a Man-in-the-Middle (MITM) attack? www.evil.com wraps 
www.container.com and steals the window.opener references it adds to its 
gadgets.
  -> Idea: Fix MITM by adding "handshake tokens" used only in establishing the 
communication channel which utilize container-private context.
    -> Doesn't work? You can wrap the original window.opener and use its 
validation method.
      -> Can't do that, because COM wrapper methods can't be replaced like JS 
ones can.
      -> Attack replaces COM wrapper with JS object
      -> Can fix this by checking type of object is COM wrapper
    -> OK, then attack: wrap validation method using more VBScript wrapper code
      -> Can't do this - we forgot that window.opener can be set by anyone but 
only *read* by the gadget
    -> What about a race condition exploiting the period of time between 
validation of the "handshake token" and setting up the channel?
      -> We're covered: just store window.opener in a local variable so 
replacement can't happen

Open topics:
* Degree of confidence that COM objects don't betray their creation context 
like JS ones do?
  -> pretty high, nobody's been able to break it, but we could use a COM expert
* What about a Flash-based transport?
  -> feasible, with a working standalone prototype... new issue to be created!

Original thread content below.

[[Michal Zalewski]]

Vbscript wrappers *probably* prevent object constructor trickery and
security context fenceposts, but there are some additional problems
associated with the fact that missing same-origin checks on .opener
are not only exploitable by the two "legitimate" parties implementing
IPC, but also can be abused to hijack or interfere with IPC sessions
of this type at any time by third-party domains.


[[Brian Eaton]]

On Mon, Jun 30, 2008 at 4:10 AM, Michal Zalewski <[EMAIL PROTECTED]> wrote:
> Vbscript wrappers *probably* prevent object constructor trickery and
> security context fenceposts, but there are some additional problems
> associated with the fact that missing same-origin checks on .opener
> are not only exploitable by the two "legitimate" parties implementing
> IPC, but also can be abused to hijack or interfere with IPC sessions
> of this type at any time by third-party domains.

I was trying to think of a way to avoid this, and I can't.  It seems
like there is going to be an opportunity for an untrusted domain to
eavesdrop on or hijack completely the communication channel.  For
example:

www.evil.com creates an iframe to www.google.com/ig

www.google.com/ig creates an iframe to <some-sensitive-gadget>, using
the window.opener trick to establish a communication path.

www.evil.com repeats the window.opener trick to hijack messages sent
from <some-sensitive-gadget>

<some-sensitive-gadget> leaks the IFPC token/RPC token/iGoogle "edit
token" to evil.com.

This isn't possible with the normal IFPC mechanism because messages
sent using URLs have some implicit authentication.  Only code in the
domain of the URL can read the info on the URL.

Cheers,
Brian


[[John Hjelmstad]]

Good point. I'd had a fuzzy notion that this would be a new attack vector, but 
didn't quite have a strong enough sense of it to conjure a potential solution.

But I think I may (?) now have one... though it would require augmenting the 
authentication token passing mechanism. Looking for your thoughts.

The idea is to add two new tokens to the RPC mechanism. Today we have rpcToken. 
I propose adding (coming up with names here) handshakeQueryToken (hqt) and 
handshakeValidationToken (hvt):

#rpcToken=abcd&hqt=efgh&hvt=ijkl

One new method would be added to the channel-creation wrapper:
ChannelCreatorObject GetChannelCreator(string handshakeQuery)

This method returns a ChannelCreatorObject (also VB) with two methods:
ChannelCreatorObject.CreateChannel(...) -> moved from the current one - 
actually establishes the bidirectional channel.
ChannelCreatorObject.GetValidationToken() -> returns HVT only if HQT was 
specified accurately

NULL could be returned as well if HQT wasn't correct.

During handshake, the gadget does:
var creator = window.opener.GetChannelCreator(hqt);
if (creator && creator.GetValidationToken && creator.GetValidationToken() === 
hvt) {
  creator.CreateChannel(...);
}

Both HQT and HVT are privileged information to both parties in the handshake, 
container and gadget. Neither can be read by a third party, nor spoofed by 
picking off a given window.opener object and reading its values.

The technique would rely on the container passing in these values as 
appropriate. Without them, the library would automatically fall back to IFPC. 
This seems like a reasonable burden to impose on a container in exchange for 
better IE performance.

Thoughts? Am I missing something?
John


[[Michal Zalewski]]

Without looking at the code (I'm about to take off, as it's pretty
late) - is the handshake expected to take place once, with further IPC
through this channel possibly taking place at a later time? If so, it
would be still possible to inject malicious handlers once a handshake
is completed, to intercept further data.

If the handshake takes place every time immediately before any data is
exchanged, and a two-way secret-based authentication takes place, with
different pairs of secrets created for every IPC channel (shared
secrets would permit cross-gadget attacks)... then it's hard to
settle. Currently, most Javascript engines do not implement
multitasking with preemption, or implement it in only a very limited
manner, so this would probably be sorta-safe in this aspect. There are
no strict guarantees, however, that while one window is busy, others
would not be able to access or modify properties creating race
conditions. It's hacky...

/mz


[[Joey Schorr]]

Hey Mike,
 
Once the channel is setup (via the handshake), then all communication is done 
via that single channel, which is stored locally in a JScript var. Even if an 
evil frame were to come along and override window.opener, it would no longer 
matter unless the gadget were reloaded and had to reestablish a new channel 
(which means we should have a new secret, etc).

Thanks,
Joey


[[Brian Eaton]]

Would this attack be feasible?

Before handshake, malicious site does this:
var oldCreator = <path-to-gadget>.window.opener.GetChannelCreator;
<path-to-gadget>.window.opener.GetChannelCreator = function(hqt) {
    var fakeCreator = {};
    fakeCreator.GetValidationToken = function() {
        return oldCreator.GetValidationToken();
    }
    fakeCreator.CreateChannel = ... malicious function ...
    return fakeCreator;
}

> During handshake, the gadget does:
> var creator = window.opener.GetChannelCreator(hqt);

   This returned the MITM fakeCreator object

> if (creator && creator.GetValidationToken && creator.GetValidationToken()
> === hvt) {
   These tests pass because the fake creator object proxies GetValidationToken

>   creator.CreateChannel(...);

   This allows the MITM object to intercept the channel.

Cheers,
Brian


[[Joey Schorr]]

Hey Brian,
 
No, as the object in window.opener is a VBScript COM wrapper. Therefore, you 
cannot selectively replace portions of the object as you would in JavaScript. 
In order to replace the GetChannelCreator method, one would have to replace the 
entire object found in window.opener. Since the secret is contained in this 
object (and only accessible to it internally), it would result in an invalid 
opener being placed in the channel.

Thanks,
Joey


[[Brian Eaton]]

I don't see what in the code prevents this attack from working.  I'm
not replacing methods on the VBScript object.  I'm replacing the
entire object, then shunting off a few function calls to it.  Where
(and how) do we check that the call is really going to the VBScript
object?  How do we prevent something else from impersonating that
object?


[[John Hjelmstad]]

Sounds like what the code really needs to do is to verify that what it's 
calling is on a VBScript object... or perhaps, it's good enough to know that 
it's not a JS object.

So:
1. Joey, know of any way we can do the former? Does (typeof VBScriptObject) 
return something unique?
2. Perhaps we could just make sure that creator.GetChannelCreator is not a JS 
function:
if (!creator.GetChannelCreator.constructor) {
  // sets constructor to null if it was set to undefined
  delete creator.GetChannelCreator.constructor;
  // resets to original value if null, otherwise does nothing
  delete creator.GetChannelCreator.constructor;
}
if (!creator.GetChannelCreator.constructor) {
  // do channel creation
}

The first block here is annoying, but takes advantage of the fact that IE 
resets a property's builtin original value on double-delete, even if it was set 
to undefined. There may be other, cleaner ways to determine not-JS-ness.

Short of this: we could mitigate, or remove, this form of attack by controlling 
handshake semantics. In particular:
1. If window.opener is set at gadget load time, the injected gadgets.rpc code 
slurps it up before malicious gadget code could.
2. We could initiate a setInterval timer polling for window.opener to be set in 
the gadget.

Each of these only shortens the window in which the original opener reference 
could be sucked up though... which isn't the best security guarantee.

--John


[[Joey Schorr]]

Comments inline.
 
Thanks,
Joey

 
On 6/30/08, John Hjelmstad <[EMAIL PROTECTED]> wrote:

    Sounds like what the code really needs to do is to verify that what it's 
calling is on a VBScript object... or perhaps, it's good enough to know that 
it's not a JS object.

    So:
    1. Joey, know of any way we can do the former? Does (typeof VBScriptObject) 
return something unique?

 
Yes, 'unknown'.

    2. Perhaps we could just make sure that creator.GetChannelCreator is not a 
JS function:
    if (!creator.GetChannelCreator.constructor) {
      // sets constructor to null if it was set to undefined
      delete creator.GetChannelCreator.constructor;
      // resets to original value if null, otherwise does nothing
      delete creator.GetChannelCreator.constructor;
    }
    if (!creator.GetChannelCreator.constructor) {
      // do channel creation
    }

 
It wouldn't be a JScript function.

    The first block here is annoying, but takes advantage of the fact that IE 
resets a property's builtin original value on double-delete, even if it was set 
to undefined. There may be other, cleaner ways to determine not-JS-ness.

    Short of this: we could mitigate, or remove, this form of attack by 
controlling handshake semantics. In particular:
    1. If window.opener is set at gadget load time, the injected gadgets.rpc 
code slurps it up before malicious gadget code could.
    2. We could initiate a setInterval timer polling for window.opener to be 
set in the gadget.

    Each of these only shortens the window in which the original opener 
reference could be sucked up though... which isn't the best security guarantee.

 
The real fact of the matter is: We need to treat this like any other 
man-in-the-middle attack. We can minimize the timeframe of it, but we should 
also have an ability to work our way around it. Off the top of my head, we 
could encrypt the traffic on both ends, which means that the worst someone 
could do is prevent traffic (i.e. replace the object and fail to deliver 
messages), although this could be mitigated against as well. I'm going to give 
some thought to a way to prevent this kind of attack without resorting to 
costly encryption.


[[John Hjelmstad]]

Ah, of course. Then typeof window.opener === "unknown", which at least forces 
the man-in-the-middle attack to itself utilize VBScript wrapping tricks.

Or, you know, we could always use IFPC to pass some handshake secret to 
initiate the faster connection ;)

--john


[[Joey Schorr]]

Something just occurred to me... The man-in-the-middle attack requires that the 
"evil" frame be able to read the object stored in the gadget's window.opener in 
order to forward calls and thus give the impression that it is valid. Catch 
is... only the gadget itself can read window.opener (I just remembered this and 
confirmed it again); all others can only write to it. So while an evil frame 
can easily overwrite the object stored in window.opener, that object will never 
be able to prove itself valid, because it won't have the shared secret 
necessary to validate itself that the container passed to the gadget via the 
URL (or IFCP). Thus, I don't believe there is a man-in-the-middle attack that 
can work, making the solution a simple addition of another secret (or even 
using the existing one).


[[Brian Eaton]]

Ah ha!  I hadn't realized that window.opener had the same write-only
property as document.location.  This is good, that's an authentication
primitive we can use.  There's still a race condition:

evil.com creates an iframe/window to container page
container page creates an iframe to gadget with the IFPC auth token
container page sets window.opener of the gadget iframe
evil.com sets window.opener of the gadget iframe
gadget calls window.opener with the IFPC auth token

I think we might be able to do something about that, though, using the
handshake that John outlined earlier, and making sure that
window.opener is only used once.

A couple of other questions:
- are there any concerns about gadgets that use window.opener getting
busted by this technique?  For example, the MySpace gadget uses it:
http://igdev.myspace.com/static/MySpaceGadget.xml
(http://igdev.myspace.com/gadget_dev/js/main.prod.js).

- how much of what window.opener gives us could we get with a
less-hacky flash-based channel?

- what kind of data are we talking about passing via this mechanism,
and how much product are we building on top of it?  window.opener is a
*bug*.  I'm concerned that it will get fixed (or the security we think
we might possibly have will get broken), and then we're going to have
to remove features from a product because they pose unacceptable
security or performance problems.  I'd prefer that we find cleaner
ways to transfer the data.

Cheers,
Brian


[[Joey Schorr]]

Hey Brian,

Comments inline.

Thanks,
Joey

 
On 7/1/08, Brian Eaton <[EMAIL PROTECTED]> wrote:

    Ah ha!  I hadn't realized that window.opener had the same write-only
    property as document.location.  This is good, that's an authentication
    primitive we can use.  There's still a race condition:

    evil.com creates an iframe/window to container page
    container page creates an iframe to gadget with the IFPC auth token
    container page sets window.opener of the gadget iframe
    evil.com sets window.opener of the gadget iframe
    gadget calls window.opener with the IFPC auth token

 
Well, if we use a token passed via the URL itself, rather than IFPC, then we 
don't need to worry about this problem. On the other hand, we can use IFPC, but 
have that confirmed by the URL token. Either way, we need to assume one secure 
channel of information, but we make that assumption today.

    I think we might be able to do something about that, though, using the
    handshake that John outlined earlier, and making sure that
    window.opener is only used once.

 
Agreed.

    A couple of other questions:
    - are there any concerns about gadgets that use window.opener getting
    busted by this technique?  For example, the MySpace gadget uses it:
    http://igdev.myspace.com/static/MySpaceGadget.xml
    (http://igdev.myspace.com/gadget_dev/js/main.prod.js).

 
That is my only major concern at the moment. We'd need to make sure we don't 
break existing code using it, but we can minimize this by putting the original 
object back in place, for example, once we have established the channel (i.e. 
save a reference, establish the channel with the container, reset the reference 
all before the Gadget-specific code runs)

    - how much of what window.opener gives us could we get with a
    less-hacky flash-based channel?

 
Same benefits, I'd imagine. The major downside is that this works on every 
version of IE (except for 8) while Flash is not guarenteed. I'm in favor of a 
multi-prong approach: Implement both, and cover all our bases.

    - what kind of data are we talking about passing via this mechanism,
    and how much product are we building on top of it?  window.opener is a
    *bug*.  I'm concerned that it will get fixed (or the security we think
    we might possibly have will get broken), and then we're going to have
    to remove features from a product because they pose unacceptable
    security or performance problems.  I'd prefer that we find cleaner
    ways to transfer the data.

 
I'd imagine people will finally like having a channel in IE that is as fast as 
FrameElement is in Moz. This doesn't solve the data transfer rate issues in 
Safari, so realistically, this won't help gadgets implement a truly universal 
"fast" communication channel. That being said, Safari 3+ has native support, so 
this does give us a fast channel in the large majority of cases.
 
In terms of security: We've been going round and round with this, and while 
we've had some speedbumbs, I'm reasonably confident we are fine. As with other 
techniques, we should give it scruitiny now and then if we find a problem 
later, fix it as well or reevaluate at that point.
 
In terms of this being a bug: We already know MS recognized it as such, because 
they have closed the loophole in IE8. Since they have already done so but not 
released a patch for IE6 or 7 that does so (that I have seen), I'm of the 
personal opinion that they don't think it important enough to close in those 
browsers. Fortunately, it is those browsers in which we need such a solution, 
so I think we'll be fine. Worst comes to worst, they close the hole, and we 
revert to other methods. It still gives us a significant benefit for existing 
browser share right now.


[[John Hjelmstad]]

Joey,

I can't believe I forgot about the window.opener read-only-by-gadget property. 
Forest, where are the trees? Seems rather important. ;)

I concur with your comments below, with a small amount to add:
* Existing uses of window.opener can largely be maintained as you said by 
restoring the original opener property (modulo timing issues)... but perhaps 
better, the gadget should just use RPC to talk to the parent page. Looks like 
the MySpace gadget uses opener to click some kind of callback, which could be 
provided by a custom RPC service.

* Brian: I've been pushing this technique as opposed to the Flash one for a few 
reasons, despite Flash also being quite fast:
  - No deployment requirements (most important of them).
  - Faster startup time.
  - Marginally faster, in the brief time I've seen it used, performance.

For what it's worth. I still would love to see the Flash-based impl put in this 
bag o' tricks, at the ready in case some funky new exploit is found with any 
given technique (frameElement and opener alike), while speeding up Safari and 
any other browsers that aren't covered by DPM/WPM/opener/FE.

--John


[[Brian Eaton]]

> Well, if we use a token passed via the URL itself, rather than IFPC, then we
> don't need to worry about this problem. On the other hand, we can use IFPC,
> but have that confirmed by the URL token. Either way, we need to assume one
> secure channel of information, but we make that assumption today.

Sorry, my terminology was confusing.  By "IFPC auth token", I meant
the random value passed on the URL.  The gadget has no way of knowing
whether a call to window.opener goes to evil.com or the legitimate
container page.  How do we solve that problem?

Cheers,
Brian


[[Joey Schorr]]

We don't need to. Here is how I envision it working:

    * Container creates a wrapper with a "secret" inside of it returned by some 
"GetSecret" method that can only be called once (just to be extra sure)
    * Container places this wrapper in window.opener and sends the secret via 
normal secure channels to the gadget
    * Gadget gets reference to the wrapper
    * Gadget calls "GetSecret" and verifies that the secret returned matches 
(thus verifying that the wrapper in fact came from the opener)
    * We don't need to worry about the evil frame, because they can't get a 
hold of the secret via the wrapper because they can't get a reference to the 
wrapper itself
    * Once the secret has been confirmed by the gadget, it can then trust the 
wrapper and setup the channel.

Thanks,
Joey


[[Brian Eaton]]

Race:
Container creates wrapper
Gadget verifies wrapper
Evil page replaces wrapper
Gadget creates channel

The verification of the wrapper and the creation of the channel need
to be atomic.


[[John Hjelmstad]]

Right, that's why I proposed that an object be returned from 
GetChannelCreator(handshakeQuery) -- the object contains both the return 
validation token and the CreateChannel method. The latter can't be replaced 
in-situ on a VBScript object, so the race can't happen. Does that work?

John


[[Joey Schorr]]

Why is there a race condition at all? Gadget code currently does and should do:
 
var opener = window.opener;
 
if (opener && typeof opener === 'unknown' && ...) {
 // Verify opener here and if valid, use the object stored
 // stored in a local private variable henceforth.
}
 
The assignment operation in Javascript is atomic (in the sense that there is no 
multithreading), so I don't see where a race condition can occur...

Thanks,
Joey


[[Brian Eaton]]

On Tue, Jul 1, 2008 at 1:15 PM, Joseph Schorr <[EMAIL PROTECTED]> wrote:
> Why is there a race condition at all? Gadget code currently does and should
> do:
>
> var opener = window.opener;
>
> if (opener && typeof opener === 'unknown' && ...) {
>  // Verify opener here and if valid, use the object stored
>  // stored in a local private variable henceforth.
> }
>
> The assignment operation in Javascript is atomic (in the sense that there is
> no multithreading), so I don't see where a race condition can occur...

That sounds right.  Code like this might be a problem:
  if (window.opener.CheckValid(code)) {
     window.opener.CreateChannel(func);
  }

But moving window.opener into a private variable seems to reduce the risk.

How certain are we that no one can tamper with a VB script object?


[[Joey Schorr]]

At the moment, I'm pretty confident, but we obviously cannot be for sure 100%. 
Even if one *could* tamper with the VBScript object, since a malicious frame 
can't read the object stored in window.opener, merely replace it wholesale, it 
shouldn't change the security of this verification method.
 
Thanks,
Joey


[[Brian Eaton]]

If you can tamper with the VBScript object it seems like we would be
risking the security of the container page.  Or am I missing
something?

Container gives VBScript to gadget
VBScript has references to container dom/javascript context
gadget tampers with VBScript to gain access to those references
gadget has access to container dom.


[[Joey Schorr]]

Yes, that was the original concern. This would only be valid if you could not 
only tamper with the wrapper object, but somehow break its encapsulation of the 
context to then pass a non-VBScript object from the parent context to the 
gadget or vice versa. The current consensus is a guarded optimism in support of 
it working. I personally believe that it is safe enough for now, but should 
obviously be monitored.


[[Brian Eaton]]

We should talk a bit about whether the callback function passed from
the gadget to the container page also needs wrapping, and if so what
kind.  And what happens when the container page calls into that
function?  Can the gadget use tricks like func.caller to poke at the
container page?

Cheers,
Brian


[[Joey Schorr]]

Hey Brian,
 
Some comments inline.
 
Thanks,
Joey

 
On 7/2/08, Brian Eaton <[EMAIL PROTECTED]> wrote:

    [+jasvir, since he and I burnt an hour today talking about this.  Jas,
    I'll forward you the thread.]

    Text LGTM.  Feel free to add the comment below.

    We should talk a bit about whether the callback function passed from
    the gadget to the container page also needs wrapping, and if so what
    kind.

 
Yes, it does. The current implementation has the gadget sending its own wrapper 
into the containers wrapper, thereby not exposing its own context to the 
container's wrapper, since it may not truly be following the rules. This way, 
each side only has a reference to the other's wrapper object, thereby 
preventing context breaking.

      And what happens when the container page calls into that
    function?

 
The container calls that function via a wrapper call.

      Can the gadget use tricks like func.caller to poke at the
    container page?

 
No, func.caller is null when called from inside a wrapper object (I've verified 
this in the past and just now once again to be absolutely sure).


> Implement window.opener-based IE transport ("NIX") in gadgets.rpc
> -----------------------------------------------------------------
>
>                 Key: SHINDIG-416
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-416
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Features (Javascript)
>            Reporter: John Hjelmstad
>            Assignee: John Hjelmstad
>         Attachments: rpc.nix-full.patch, rpc.nix-refactor.diff
>
>
> Joey Schorr and I have been developing a technique for high-speed 
> cross-domain message passing in IE6 and IE7 that exploits an odd property: 
> for a given window object, window.opener can be set by any party, but only 
> read by the controlling window.
> The message-passing technique is to pass a "channel creation" object from the 
> container, across domain boundaries, to the gadget. The gadget uses this 
> object to establish a bi-directional communication channel used by all 
> subsequent gadgets.rpc calls.
> We can't pass a JavaScript object through window.opener, however, because 
> doing so enables access to the passing agent's full window context. Eg. if 
> the container sets window.opener = function() { ... }, then the gadget can 
> access the entire container page with:
> var containerWindow = (new window.opener.constructor("return window;"))();
> Instead, we pass a VBScript (COM) wrapper through window.opener, since COM 
> objects don't have this property. The gadget passes back such a wrapper as 
> well, ensuring mutually isolated contexts.
> Patch forthcoming.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to