Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent window.parent access from iframe #534

Closed
danyocom opened this issue Mar 12, 2013 · 41 comments
Closed

prevent window.parent access from iframe #534

danyocom opened this issue Mar 12, 2013 · 41 comments
Milestone

Comments

@danyocom
Copy link

Hello,
I am looking into developing an application using node-webkit. One thing this application will do is load untrusted content (html/js) (served from a webserver running on localhost) into an iframe. I noticed that no matter what sandbox attributes I set on the iframe (but always allowing js), the javascript inside the Iframe can call window.parent.someNodeJsCommand().

Is there a way to prevent the iframe from being able to access window.parent? I do need to communicate between the iframe and the parent application, and was planning on using window.postMessage for that.

Thanks,
Dan

@ibdknox
Copy link
Contributor

ibdknox commented Apr 11, 2013

@rogerwang any thoughts on this? This is a particularly dangerous security hole. I wanted to be able to use the iframes as a mini-browser, but given that the frame can access window.parent, someone could delete your entire harddrive just by putting a window.parent.require('fs').rmdir in there.

@ibdknox
Copy link
Contributor

ibdknox commented Apr 11, 2013

For what it's worth, the ideal in my case would be that the parent window can access and mess around with the iframe, but that the iframe could never touch the parent window. If there was a way to remove window.parent and window.top from the iframe's context, in theory that would address the issue as far as I know.

@ibdknox
Copy link
Contributor

ibdknox commented Apr 11, 2013

ah, looks like you'd also need to nuke window.frameElement

@rogerwang
Copy link
Member

There is some bug in NW's handling for cross domain checking. I'll fix it in 0.5.0. After that I believe such need can be fulfilled?

@ibdknox
Copy link
Contributor

ibdknox commented Apr 12, 2013

With the cross domain check in place, would I still be able to access the contents of the frame from the parent? i.e. window.frames["my-iframe"].document.head.appendChild(...); kind of stuff

@rogerwang
Copy link
Member

Are you suggesting the access should be unidirectional? I can see it when I try to fix it.

@ibdknox
Copy link
Contributor

ibdknox commented Apr 12, 2013

Yeah, that's what I'm hoping for. Being able to access the child from the parent window, but ensuring that the child can't access the parent. This allows me to do whatever I want (i.e. capturing certain keys) within the frame, while making sure that child frame can't delete my harddrive :)

@danyocom
Copy link
Author

I would like for postMessage to work bidirectionally. AKA the child iframe can call a postMessage on the parent, and the parent can call postMessage on its child iframe.

@rogerwang
Copy link
Member

@danyocom Sure. postMessage should remain untouched behavior as the standard.

rogerwang added a commit to rogerwang/WebKit_trimmed that referenced this issue Apr 14, 2013
@ibdknox
Copy link
Contributor

ibdknox commented Apr 25, 2013

One thing I've run into with this is that sites that use framebusting are causing all sorts of trouble. @rogerwang any thoughts on making it such that iframes with nwdisable have window.top and window.parent set to the iframe's window? This would make the frame seem like it's the root and prevent all that nonsense.

That being said, I have no idea how hard that would necessarily be - if you have thoughts on some other way I can get around it, I'm happy to hear them!

@rogerwang
Copy link
Member

It could be done. But it'd be better with another flag attribute, since some app may want to access window.top and window.parent. What do you think? Any suggestion to the name of the flag?

@ibdknox
Copy link
Contributor

ibdknox commented Apr 25, 2013

Yeah another attribute seems fine. Maybe something like nwRootFrame or nwTopFrame?

@Mithgol
Copy link
Contributor

Mithgol commented Apr 25, 2013

nwfaketop

@twrodriguez
Copy link

@rogerwang How soon do you think you could patch this functionality in? I've had a little luck myself, but lots of unprompted quitting without any hints as to what's wrong. I'd much rather yield to the maintainer :)

Also, I'll second nwfaketop. That's much better than the one I've been using: nwisolatedsandbox

@rogerwang
Copy link
Member

I need to look into it a little bit first, but it shouldn't be hard.

Could any of you provide some test case so I'll be sure it works for you.

@twrodriguez
Copy link

Yeah, I wouldn't think so, but I just don't know Webkit well enough to know where to look. I know it's at least more nuanced than letting FrameTree::parent() return null if the attribute is there. Patching that passed a rudimentary test case (just try showing twitter.com in an iframe, and clicking a google search result in an iframe), but crashed in my app. I'll write up some thorough test cases in the morning.

@Mithgol
Copy link
Contributor

Mithgol commented Apr 25, 2013

@rogerwang

The test case is the following:

nwfaketop/package.json

{
  "name": "nwfaketop",
  "main": "nwfaketop.html",
  "window": {
    "width": 600,
    "height": 400,
    "position": "center",
    "toolbar": false,
    "title": "Starting nwfaketop...",
    "resizable": true
  }
}

nwfaketop/nwfaketop.html

<!doctype html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Testing nwfaketop</title>
  </head>
  <body>
    <b>You should be able to see Twitter in an iframe below:</b>
    <iframe src="https://twitter.com" style="width: 80%;" nwdisable nwfaketop>
    </iframe>
  </body>
</html>

Command line:
nw nwfaketop

Expected behaviour: Twitter in iframe.

Current behaviour: Twitter realizes it's not in the top frame → Twitter takes the whole window.

I dunno if Twitter breaks free of the nwdisable restrictions when does this. (After all, the iframe holding that nwdisable attribute is killed.) If it does, we also face some security risk. I guess @ibdknox was totally right to suggest that nwdisable restrictions should also impose nwfaketop on the same iframe (after all, I have a little trouble imagining why someone would want the opposite).

rogerwang added a commit to rogerwang/WebKit_trimmed that referenced this issue Apr 25, 2013
With 'nwfaketop' attribute window.parent and window.top
would return the iframe object rather than the parent frame
and the top frame.

See nwjs/nw.js#534
@rogerwang
Copy link
Member

The previous commit passed @Mithgol 's test case. Have fun.

@ibdknox
Copy link
Contributor

ibdknox commented Apr 25, 2013

@rogerwang you are awesome :) Thanks!

@twrodriguez
Copy link

@rogerwang

While that patch will work for Twitter (bravo, by the way, I wish I knew the codebase well enough to have done it that quickly!), clicking on a search result from DuckDuckGo will still break out of the frame. Sandboxing the iframe without "allow-top-navigation" will stop it breaking out, but also stop the ability to navigate their searches (as well as Google's) due to their targetting of "_top".

It would also probably be a good idea to plug window.frameElement, as a site could potentially use it to detect and hide its content. There's also the matter of the property location.ancestorOrigins added by Chrome.

If you fix the DuckDuckGo hole, then you can probably just document "If you sandbox an nwfaketop iframe, consider including 'allow-top-navigation' permissions for a smoother user experience."

Here's some test-cases for all the methods I've talked about (Thanks for the template @Mithgol):

nwfaketop/package.json

{
  "name": "nwfaketop",
  "main": "nwfaketop.html",
  "window": {
    "width": 600,
    "height": 400,
    "position": "center",
    "toolbar": true,
    "title": "Starting nwfaketop...",
    "resizable": true
  }
}

nwfaketop/nwfaketop_targettests.html

<!doctype html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Testing Anchor Target Access</title>
  </head>
  <body>
    <a href="nwfaketop_targettests.html" target="_top">Test target="_top"</a><br/>
    <a href="nwfaketop_targettests.html" target="_parent">Test target="_parent"</a>
  </body>
</html>

nwfaketop/nwfaketop_windowproptests.html

<!doctype html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Testing Property Access</title>
    <script>
    try {
      if(window.top !== window)
        document.write("window.top Failure<br />");

      if(window.parent !== window)
        document.write("window.parent Failure<br />");

      if(window.frameElement !== null)
        document.write("window.frameElement Failure<br />");

      var l = 0;
      for(var tempWin = window; tempWin != top; tempWin = tempWin.parent) { ++l; }
      if(window.location.ancestorOrigins.length !== l)
        document.write("window.location.ancestorOrigins.length Failure<br />");

      if(document.documentElement.innerText === "")
        document.write("Success");
    }
    catch(e) { document.write("Exception Thrown Failure<br />"); }
    </script>
  </head>
  <body></body>
</html>

nwfaketop/nwfaketop.html

<!doctype html>
<html>
  <head>
    <meta charset="utf-8">
    <title>Testing nwfaketop</title>
  </head>
  <body>
    <b>You should be able to see Twitter in an iframe below:</b><br />
    <iframe src="https://twitter.com" style="width: 80%;" nwdisable nwfaketop>
    </iframe><br />

    <b>You should be able to click on a search result from Google and have it navigate within the iframe below:</b><br />
    <iframe src="https://www.google.com/search?q=foobar" style="width: 80%;" nwdisable nwfaketop>
    </iframe><br />

    <b>You should be able to click on a search result from DuckDuckGo and have it navigate within the iframe below:</b><br />
    <iframe src="https://duckduckgo.com/?q=foobar" style="width: 80%;" nwdisable nwfaketop>
    </iframe><br />

    <b>This tests the following properties: "frameElement", "parent", "top", "location.ancestorOrigins"</b><br />
    <iframe src="nwfaketop_windowproptests.html" style="width: 80%;" nwdisable nwfaketop>
    </iframe><br />

    <b>This tests target="_parent" and target="_top" access. Clicking on the links shouldn't navigate the whole window.</b><br />
    <iframe src="nwfaketop_targettests.html" style="width: 80%;" nwdisable nwfaketop>
    </iframe><br />
  </body>
</html>

Command line:
nw nwfaketop

Expected behaviour:

  • Twitter in iframe 1
  • Google search result navigation stays in iframe 2
  • DuckDuckGo search result navigation stays in iframe 3
  • 4th iframe should have a document.body.innerHtml === "Success" on load.
  • 5th iframe links should not cause the whole window to navigate

Current behaviour:

  • Twitter realizes it's not in the top frame → Twitter takes the whole window. (Fixed by 12d8e15)
  • Google navigates the top window to the search result (Fixed by 12d8e15)
  • DuckDuckGo navigates the top window to the search result
  • 4th iframe: document.documentElement.innerHtml === "window.frameElement Failure<br />window.location.ancestorOrigins.length Failure<br />"
  • Links in 5th iframe navigate whole window

@Mithgol I can confirm that indeed it does break out of the nwdisable restrictions. Definitely a security risk if nwfaketop isn't included with nwdisable.

@rogerwang
Copy link
Member

@twrodriguez , cool and it's very helpful! I'll try to fix those before 0.5.1 release.

@rogerwang
Copy link
Member

@twrodriguez there is a little error in the case under Chrome:

Exception Thrown Failure
TypeError: Cannot read property 'innerText' of null

@ibdknox
Copy link
Contributor

ibdknox commented Apr 26, 2013

You can either move the script tag to the bottom of the body tag, or change it to document.documentElement.innerText and it should be ok.

rogerwang added a commit to rogerwang/WebKit_trimmed that referenced this issue Apr 26, 2013
The following should be emulated as a top level window:

"_top" navigation target
window.frameElement
location.ancestorOrigins

see the comments in nwjs/nw.js#534
@Mithgol
Copy link
Contributor

Mithgol commented Apr 27, 2013

@rogerwang May I ask for the nwfaketop feature to be merged with the earlier nwdisable? Otherwise we are going to still have the same counterintuitive situation where any remote application is able to break free from its nwdisable restrictions by being a framebuster. Also, if nwfaketop and nwdisable are separate, then their combination (nwdisable nwfaketop) is suddenly going to become a new security default for normal frames, but that's slightly too long to type and may bring its end users some trouble in the future (such as typos or one of the two attributes accidentally missing).

@rogerwang
Copy link
Member

@Mithgol It seems should be merged with nwdisable. Before that I'd like to find out how the apps is able to break from the child frame. In the meantime I added a notice in the security page to advice using the 2 attributes together.

@twrodriguez
Copy link

Just built with 0.5.1, works beautifully. Thanks for your quick work @rogerwang!

@twrodriguez
Copy link

More news regarding working with fake-top iframes. There's two more issues I've discovered, one semi-major (security-related), and one minor:

  • A Ctrl+Click (Cmd+Click on OSX), or middle-mouse-button-click on a link will bypass the nwfaketop and nwdisable attributes, replacing the top-most window with the contents of the link, and also exposing nodejs to the linked site. I'm working around this right now by running a master Window that monitors the main window's location object, and closes it if it notices a protocol other than file:. But, this still exposes nodejs until the loaded event is fired.
  • Clicks on links with target="_blank" will open in a frameless new window (which can be rather troublesome for a user to close, nevermind the poor user experience). This is, of course, correct behavior along the lines of the current window.open() in an nwdisable iframe. I'm working around this right now by scanning all anchor links for target="_blank" and replacing them with target="_self", but it's rather hard to control window.open() behavior from a foreign script.

In light of these, I'd like to propose a new event: blankwindow. The default action, of course, would be opening a new window (with a frame? without node?). Attaching an event listener would then allow the developer to call event.preventDefault() and handle the request as they see fit.

I'm sure I haven't thought enough about this, so please weigh in with your comments.

@rogerwang
Copy link
Member

Please see the doc for the new new-win-policy event for new window handling, and make comment if you feel anything need to be changed.

https://github.com/rogerwang/node-webkit/wiki/Window#new-win-policy
It will be available in the upcoming 0.9.0 release.

@rogerwang
Copy link
Member

The new-win-policy event now works with middle and ctrl click in 0.9.2.

@ghost
Copy link

ghost commented Feb 21, 2014

parent.postMessage() does not appear to be working bi-directionally with nwfaketop, I thought it was said in these comments that postMessage would still work? Is there a workaround? (sending messages from iframe to parent)

@krittermedia
Copy link

rogerwang
all new to this cool node webkit stuff, but I'm trying to use it display a mini browser (iframe) within a start page and allow use to link around to where ever and still close the (hide it) and go back to the start page.

All good, but some links that point to Facebook or Twitter break out of my iframe and kill the user experience.

I've added the flag to the iframe for nwdisable nwfaketop but that does not seem to help.

Is there a command that I'm missing....thanks

@dhu77man
Copy link

dhu77man commented Nov 1, 2014

Is there any way to get the title of the document a iframe is viewing?

@brettz9
Copy link

brettz9 commented Nov 12, 2014

Simple question if I may---is it possible to constrain what an iframe can access of its parent while still listen in on it from the parent?

@demian85
Copy link

Can someone please tell me how to embed an external web app into a node-webkit window without using an iframe? I just want the following in the package.json:

{
"main": "http://mysite.com"
}

Is that possible?

@EthraZa
Copy link

EthraZa commented Nov 14, 2014

main: index.js

index.js: <script> window.open('http://mysite.com', '_self'); </script>

Atte.

Allan Brazute Alves
Analista de Sistemas
GHSix Serviços de Internet
+55 (11) 2967-6868
allan@ghsix.com.br / www.ghsix.com.br


2014-11-14 18:06 GMT-02:00 Demián Rodriguez notifications@github.com:

Can someone please tell me how to embed an external web app into a
node-webkit window without using an iframe? I just want the following in
the package.json:

{
"main": "http://mysite.com"
}

Is that possible?


Reply to this email directly or view it on GitHub
#534 (comment)
.

@retorquere
Copy link

@EthraZa if I do that, it just displays the text of the script (using 0.11.0)

@retorquere
Copy link

@demian85 the following works for me, but in light of the discussion above it doesn't seem to be the safe thing to do:

<!DOCTYPE html>
<html>
  <head>

  </head>
  <body>
    <script>
    window.location.href = "http://mysite.com";
    </script>
  </body>
</html>

@aymanar
Copy link

aymanar commented Jul 26, 2015

If the target for an anchor tag is something other than "_blank" the "new-win-policy" does not fire. So there is still an issue, it replaces the entire nwjs container (i.e. especially with ctrl+clicking a link)

@baconbrad
Copy link

On the original topic this is easier than you guys think.

javascript
var iframe = document.getElementById("your-iframe-id");
iframe.contentWindow.window.parent = "";

iframe.addEventListener('onload', function() {
iframe.contentWindow.window.parent = "";
});


What this does is nuke the iframe's (with the id value of "your-iframe-id") window.parent. And the listener renukes if if they refresh the iframe.

@brettz9
Copy link

brettz9 commented Jul 28, 2015

Not fully sure, but I think one may also need to deal with window properties such as top and frameElement.

@aymanar
Copy link

aymanar commented Jul 28, 2015

@baconface This doesn't the problem of ctrl-clicking or shift-clicking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests