Bug 53244 (CVE-2011-1194)

Created attachment 80338 [details] 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?