Issue 4586 - Prevent access to shadowRoot via contentWindow

Issue 4586 - Prevent access to shadowRoot via contentWindow

This improves on Dave's previous fix for this issue.

Currently we're wrapping Element.prototype.shadowRoot. With this fix we also wrap HTMLIFrameElement.prototype.contentWindow to prevent access to the original shadowRoot via that route. We also have to hold on to references to certain APIs like Object.defineProperty and HTMLIFrameElement.prototype.sandbox, just in case those are overridden by the site to bypass us.

I have tested this on Chrome 49 as well as the latest Chrome and it works fine.

[Manish Jethani, 2017-04-27 21:19:50]

Patch Set 1: Prevent access to shadowRoot via contentWindow

Please see the description.

[kzar, 2017-04-28 09:01:11]

Uh this issue is assigned to me and I'm actually working on it at the moment :/
(Yea, there was a while there where it got sidetracked for ESLint etc.)

Thing is the problem of preventing access to the shadowRoot is real, but is
becoming less important. What worries me is that the technique can also be used
for access to the other unwrapped APIs like WebSocket and RTCPeerConnection.

[Manish Jethani, 2017-04-28 10:42:46]

On 2017/04/28 09:01:11, kzar wrote:
> Uh this issue is assigned to me and I'm actually working on it at the moment
:/

That's cool, I was just playing around, it's a good way for me to learn, and I
got particularly curious about this one :)

For what it's worth, your original fix seems fine to me. I was able to prevent
the crash on Chrome 49 by replacing this line:

  window.WebSocket = WrappedWebSocket.bind();

With this:

  let boundWrappedWebSocket = WrappedWebSocket.bind();
  Object.defineProperty(window, "WebSocket", {
    configurable: true, get()
    {
      return boundWrappedWebSocket;
    }
  });

The problem seems to be that window.WebSocket and all other such properties
aren't supposed to be enumerable. When we simply assign it that way, it becomes
enumerable, and for some reason this causes the page to crash.

[Manish Jethani, 2017-05-10 09:19:42]

Closed.