Bug 33277

Summary:

HTML5 iframe sandbox bypass of window.top.location navigation via <form target="_top">

Product:

WebKit

Reporter:

Chris Evans <scarybeasts>

Component:

Frames

Assignee:

Sam Weinig <sam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, darin, eric, patrik.j.persson, sam

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

OS:

All

Bug Depends on:

33659

Bug Blocks:

Attachments:

Description	Flags
LayoutTest	none
Patch	commit-queue: commit-queue-

Chris Evans

Reported 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).

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Evans

Comment 1 2010-01-06 16:58:37 PST

cc: author of iframe sandbox attribute (thanks for implementing it)!

David Kilzer (:ddkilzer)

Comment 2 2010-01-06 17:16:30 PST

<rdar://problem/7517003>

Darin Adler

Comment 3 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?

Adam Barth

Comment 4 2010-01-06 17:26:32 PST

Created attachment 46011 [details] LayoutTest Here's a LayoutTest for the issue.

Chris Evans

Comment 5 2010-01-06 17:27:05 PST

Darin - agreed!

Sam Weinig

Comment 6 2010-01-06 19:18:41 PST

I have a fix for this. I'll take the bug.

Chris Evans

Comment 7 2010-01-27 18:20:36 PST

Was it a simple fix or did complexities arise?

Adam Barth

Comment 8 2010-01-27 23:48:44 PST

@Sam, I can fix this if you've got other things on your plate.

Adam Barth

Comment 9 2010-02-12 23:27:23 PST

Created attachment 48699 [details] Patch

Adam Barth

Comment 10 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?

Darin Adler

Comment 11 2010-02-13 21:23:43 PST

Comment on attachment 48699 [details] Patch Fix is fine, r=me. No need to wait on Sam.

WebKit Commit Bot

Comment 12 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>

WebKit Commit Bot

Comment 13 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

Adam Barth

Comment 14 2010-02-15 07:37:59 PST

Looks like this was landed, but CCing Eric because of the strange commit-bot error.

Eric Seidel (no email)

Comment 15 2010-02-17 12:27:18 PST

That looks like bug 33659.

Note You need to log in before you can comment on or make changes to this bug.