Bug 36732

Summary: sandboxed iframes from same origin should not be granted notification permission of the parent frame unless allow-same-origin is specified
Product: WebKit Reporter: Rafael Weinstein <rafaelw>
Component: WebCore Misc.Assignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, mkwst, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
html test
none
Patch
none
Patch
none
Patch none

Rafael Weinstein
Reported 2010-03-28 13:36:00 PDT
<iframe sandbox="allow-scripts"> from the same origin as the host page should be restricted from webkitNotifications.requestPermission(), webkitNotification.createNotification() & webkitNotification.createHTMLNotification unless allow-same-origin is specified. The underlying issue is that isUnique() is not being observed. Note that https://bugs.webkit.org/show_bug.cgi?id=36625 changes the interface of NotificationPresenter to pass the url rather than the security origin, so clients will not longer be able to observe the bit. Per discussion with abarth on #webkit, the preferred approach will be to have the check of isUnique() take place prior to calling into the clients NotificationPresenter. Allow per discussion with abarth on #webkit, I set about creating an (if checked in, failing) layout test, but the notifications layout tests do not yet support granting permission via the layoutController and are disabled, so I've attached a simple html test that should be easily adapted to a layout test.
Attachments
html test (906 bytes, application/x-zip-compressed)
2010-03-28 13:37 PDT, Rafael Weinstein 		no flags
Details
Patch (8.15 KB, patch)
2010-05-03 17:27 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (8.14 KB, patch)
2010-05-03 17:30 PDT, Adam Barth 		no flags
DetailsFormatted DiffDiff
Patch (3.65 KB, patch)
2012-12-29 13:45 PST, Mike West 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Rafael Weinstein
Comment 1 2010-03-28 13:37:54 PDT
Created attachment 51866 [details] html test This test loads the same document twice as an iframe, once with sandbox="allow-scripts" and once with sandbox="allow-scripts allow-same-origin". If the parent frame is granted notification permission, only the allow-same-origin iframe should report that it has been granted permission as well.
Adam Barth
Comment 2 2010-05-03 16:32:06 PDT
Looking now.
Adam Barth
Comment 3 2010-05-03 17:27:07 PDT
Created attachment 54978 [details] Patch
Adam Barth
Comment 4 2010-05-03 17:28:50 PDT
I don't know how to build with this feature turned on, but this is how I would fix it if I could. I also did some misc cleanup while I was trying to understand the code. Would someone who knows how to build/test this feature be willing to see if this code actually compiles and to write a test? Many thanks.
Adam Barth
Comment 5 2010-05-03 17:30:26 PDT
Created attachment 54979 [details] Patch
Mike West
Comment 6 2012-12-29 13:41:43 PST
It looks like this was fixed in https://bugs.webkit.org/show_bug.cgi?id=79704 That patch didn't add a test for sandboxed frames, however. I'll put one together to close this bug out.
Mike West
Comment 7 2012-12-29 13:45:31 PST
Created attachment 180944 [details] Patch
Adam Barth
Comment 8 2013-01-02 11:10:53 PST
Comment on attachment 180944 [details] Patch Thanks for the test.
WebKit Review Bot
Comment 9 2013-01-02 11:17:03 PST
Comment on attachment 180944 [details] Patch Clearing flags on attachment: 180944 Committed r138624: <http://trac.webkit.org/changeset/138624>
WebKit Review Bot
Comment 10 2013-01-02 11:17:07 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.