[Webkit-unassigned] [Bug 53244] New: A user gesture bug which can bypass popup blocker using iframe SRC

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53244

           Summary: A user gesture bug which can bypass popup blocker
                    using iframe SRC
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: PC
        OS/Version: Mac OS X 10.5
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: WebCore Misc.
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: jnd at chromium.org
                CC: abarth at webkit.org, cevans at google.com


Created an attachment (id=80338)
 --> (https://bugs.webkit.org/attachment.cgi?id=80338&action=review)
test case1

According to the description in crbug.com/70885. the following code can bypass popup blocker.
(1) <iframe src="javascript:window.open('http://www.google.com' target=_new>pop</iframe>
(2) <iframe src="javascript:window.open('http://www.google.com','','height=200,width=200' target=_new>pop</iframe>

The bug reporter did a investigation and found it's because sourceURL->isNull()==true when window.open is used with iframe, the following is the code snippet.
bool ScriptController::processingUserGesture() {
  ...
  if (sourceURL && sourceURL->isNull() && !activeProxy->timerCallback()) {
  // This is the <a href="javascript:window.open('...')> case ->  we let it through.
        return true;
  }


I was curious why sourceURL->isNull() was true, it should be "about:blank" when loading the iframe, so I did my investigation and the following is my analysis.

According to https://svn.webkit.org/changeset/35050 and https://svn.webkit.org/changeset/55674, Darin and Adam introduced a way to look at the sourceURL to figure out whether we're running a script tag or a hyperlink. It does guarantee that the empty sourceURL does mean the we are running the scripts from a hyper-link. Please refer to 
https://svn.webkit.org/browser/trunk/WebCore/bindings/ScriptControllerBase.cpp#L49
https://svn.webkit.org/browser/trunk/WebCore/bindings/js/ScriptController.cpp#L497
So the protocols is all user-initiated javascript URLs are handled with forceUserGesture true in WebKit. For example, in WebFrameImpl::loadJavaScriptURL (WebKit/chromium port), it passed forceUserGesture=true to ScriptController::executeScript(const String& script, bool forceUserGesture, ShouldAllowXSS shouldAllowXSS) to explicitly tell WebCore we are running the scripts from a hyper-link.

In this bug, the iframe SRC is not a hyper-link, so we should not set forceUserGesture parameter as true when calling ScriptController::executeScript, but currently it did happen,
In function SubframeLoader::requestFrame, it calls ScriptController::executeIfJavaScriptURL to execute the SRC scrip. 
In function ScriptController::executeIfJavaScriptURL (line 93), it use the return value of processingUserGesture() to set forceUserGesture parameter, since the iframe is loaded by main frame, there is not active frame, the processingUserGesture() returns true, so ScriptController::executeScript set the sourceURL to NULL.

For my perspective, the "forceUserGesture" parameter should be only used when you are definitely sure that the running script is from a hyper-link, what is why we add this parameter. We should not set it according to current gesture status from processingUserGesture(), they are not relevant. Even the "forceUserGesture" is set to false, the other functions still can get right gesture status from ScriptController::processingUserGesture().

In here, I propose two ways which changes ScriptController::executeIfJavaScriptURL to fix this bug.
(1)always set "forceUserGesture" to false in ScriptController::executeIfJavaScriptURL. For situation of running a script from a hyperlink, we already handle it in WebKit ports (for example, WebFrameImpl::loadJavaScriptURL)
(2)add a new boolean-type parameter, useCurrentGestureStatus, to indicates whether we want to use current gesture status to set the "forceUserGesture" parameter. then in SubframeLoader::requestFrame, call ScriptController::executeIfJavaScriptURL(url, false, ...)

The webkit used not to have this bug, it's because we explicitly set the forceUserGesture to false in old code, you can refer to https://bugs.webkit.org/attachment.cgi?id=70970&action=diff from bug 47777. In old code, ScriptController::executeIfJavaScriptURL set userGesture to false as default value, and the SubframeLoader::requestFrame used the default value.

@Adam, what do you think?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.