Changeset 183781 in webkit


Ignore:
Timestamp:
May 4, 2015 4:58:32 PM (9 years ago)
Author:
Chris Dumez
Message:

Crash at com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::createWindow + 185
​https://bugs.webkit.org/show_bug.cgi?id=144597
<rdar://problem/20361579>

Reviewed by Andreas Kling.

Source/WebCore:

Test: fast/dom/Window/window-open-activeWindow-null-frame.html

In our implementation of window.open(), we make sure that the window
which window.open() is called has a frame. However, we did not have the
same check for the activeDOMWindow (i.e. the lexicalGlobalObject) causing
us to crash in WebCore::createWindow() when dereferencing it.

This patch updates WebCore::createWindow() takes a reference to the
openerFrame instead of a pointer to make it clear the implementation
expects it to be non-null. A null check is then added for the frame
at the call site: DOMWindow::createWindow().

  • inspector/InspectorFrontendClientLocal.cpp:

(WebCore::InspectorFrontendClientLocal::openInNewTab):

  • loader/FrameLoader.cpp:

(WebCore::isDocumentSandboxed):
(WebCore::FrameLoader::submitForm):
(WebCore::createWindow):
Take a reference to openerFrame instead of a pointer as the
implementation expects it to be non-null.

  • loader/FrameLoader.h:
  • page/DOMWindow.cpp:

(WebCore::DOMWindow::createWindow):
Add null check for activeFrame before passing it to
WebCore::createWindow().

LayoutTests:

Add a layout test to cover the case where window.open() is called on a
window that is different than the activeDOMWindow and where the
activeDOMWindow does not have a frame.

  • fast/dom/Window/resources/test-frame.html: Added.
  • fast/dom/Window/window-open-activeWindow-null-frame-expected.txt: Added.
  • fast/dom/Window/window-open-activeWindow-null-frame.html: Added.
Location:
trunk
Files:
3 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r183777 r183781  
     12015-05-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Crash at com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::createWindow + 185
     4        https://bugs.webkit.org/show_bug.cgi?id=144597
     5        <rdar://problem/20361579>
     6
     7        Reviewed by Andreas Kling.
     8
     9        Add a layout test to cover the case where window.open() is called on a
     10        window that is different than the activeDOMWindow and where the
     11        activeDOMWindow does not have a frame.
     12
     13        * fast/dom/Window/resources/test-frame.html: Added.
     14        * fast/dom/Window/window-open-activeWindow-null-frame-expected.txt: Added.
     15        * fast/dom/Window/window-open-activeWindow-null-frame.html: Added.
     16
    1172015-05-04  Simon Fraser  <simon.fraser@apple.com>
    218

  • trunk/Source/WebCore/ChangeLog

    r183778 r183781  
     12015-05-04  Chris Dumez  <cdumez@apple.com>
     2
     3        Crash at com.apple.WebKit.WebContent at com.apple.WebCore: WebCore::createWindow + 185
     4        https://bugs.webkit.org/show_bug.cgi?id=144597
     5        <rdar://problem/20361579>
     6
     7        Reviewed by Andreas Kling.
     8
     9        Test: fast/dom/Window/window-open-activeWindow-null-frame.html
     10
     11        In our implementation of window.open(), we make sure that the window
     12        which window.open() is called has a frame. However, we did not have the
     13        same check for the activeDOMWindow (i.e. the lexicalGlobalObject) causing
     14        us to crash in WebCore::createWindow() when dereferencing it.
     15
     16        This patch updates WebCore::createWindow() takes a reference to the
     17        openerFrame instead of a pointer to make it clear the implementation
     18        expects it to be non-null. A null check is then added for the frame
     19        at the call site: DOMWindow::createWindow().
     20
     21        * inspector/InspectorFrontendClientLocal.cpp:
     22        (WebCore::InspectorFrontendClientLocal::openInNewTab):
     23        * loader/FrameLoader.cpp:
     24        (WebCore::isDocumentSandboxed):
     25        (WebCore::FrameLoader::submitForm):
     26        (WebCore::createWindow):
     27        Take a reference to openerFrame instead of a pointer as the
     28        implementation expects it to be non-null.
     29
     30        * loader/FrameLoader.h:
     31        * page/DOMWindow.cpp:
     32        (WebCore::DOMWindow::createWindow):
     33        Add null check for activeFrame before passing it to
     34        WebCore::createWindow().
     35
    1362015-05-04  Dean Jackson  <dino@apple.com>
    237

  • trunk/Source/WebCore/inspector/InspectorFrontendClientLocal.cpp

    r183498 r183781  
    215215    bool created;
    216216    WindowFeatures windowFeatures;
    217     RefPtr<Frame> frame = WebCore::createWindow(&mainFrame, &mainFrame, request, windowFeatures, created);
     217    RefPtr<Frame> frame = WebCore::createWindow(mainFrame, &mainFrame, request, windowFeatures, created);
    218218    if (!frame)
    219219        return;

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r183698 r183781  
    170170// API simpler.
    171171//
    172 static bool isDocumentSandboxed(Frame* frame, SandboxFlags mask)
    173 {
    174     return frame->document() && frame->document()->isSandboxed(mask);
     172static bool isDocumentSandboxed(Frame& frame, SandboxFlags mask)
     173{
     174    return frame.document() && frame.document()->isSandboxed(mask);
    175175}
    176176
    … …  
    356356        return;
    357357
    358     if (isDocumentSandboxed(&m_frame, SandboxForms)) {
     358    if (isDocumentSandboxed(m_frame, SandboxForms)) {
    359359        // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
    360360        m_frame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Blocked form submission to '" + submission->action().stringCenterEllipsizedToLength() + "' because the form's frame is sandboxed and the 'allow-forms' permission is not set.");
    … …  
    34143414}
    34153415
    3416 PassRefPtr<Frame> createWindow(Frame* openerFrame, Frame* lookupFrame, const FrameLoadRequest& request, const WindowFeatures& features, bool& created)
     3416PassRefPtr<Frame> createWindow(Frame& openerFrame, Frame* lookupFrame, const FrameLoadRequest& request, const WindowFeatures& features, bool& created)
    34173417{
    34183418    ASSERT(!features.dialog || request.frameName().isEmpty());
    … …  
    34213421
    34223422    if (!request.frameName().isEmpty() && request.frameName() != "_blank") {
    3423         if (RefPtr<Frame> frame = lookupFrame->loader().findFrameForNavigation(request.frameName(), openerFrame->document())) {
     3423        if (RefPtr<Frame> frame = lookupFrame->loader().findFrameForNavigation(request.frameName(), openerFrame.document())) {
    34243424            if (request.frameName() != "_self") {
    34253425                if (Page* page = frame->page())
    … …  
    34333433    if (isDocumentSandboxed(openerFrame, SandboxPopups)) {
    34343434        // FIXME: This message should be moved off the console once a solution to https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
    3435         openerFrame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Blocked opening '" + request.resourceRequest().url().stringCenterEllipsizedToLength() + "' in a new window because the request was made in a sandboxed frame whose 'allow-popups' permission is not set.");
     3435        openerFrame.document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Blocked opening '" + request.resourceRequest().url().stringCenterEllipsizedToLength() + "' in a new window because the request was made in a sandboxed frame whose 'allow-popups' permission is not set.");
    34363436        return nullptr;
    34373437    }
    … …  
    34393439    // FIXME: Setting the referrer should be the caller's responsibility.
    34403440    FrameLoadRequest requestWithReferrer = request;
    3441     String referrer = SecurityPolicy::generateReferrerHeader(openerFrame->document()->referrerPolicy(), request.resourceRequest().url(), openerFrame->loader().outgoingReferrer());
     3441    String referrer = SecurityPolicy::generateReferrerHeader(openerFrame.document()->referrerPolicy(), request.resourceRequest().url(), openerFrame.loader().outgoingReferrer());
    34423442    if (!referrer.isEmpty())
    34433443        requestWithReferrer.resourceRequest().setHTTPReferrer(referrer);
    3444     FrameLoader::addHTTPOriginIfNeeded(requestWithReferrer.resourceRequest(), openerFrame->loader().outgoingOrigin());
    3445 
    3446     Page* oldPage = openerFrame->page();
     3444    FrameLoader::addHTTPOriginIfNeeded(requestWithReferrer.resourceRequest(), openerFrame.loader().outgoingOrigin());
     3445
     3446    Page* oldPage = openerFrame.page();
    34473447    if (!oldPage)
    34483448        return nullptr;
    34493449
    3450     Page* page = oldPage->chrome().createWindow(openerFrame, requestWithReferrer, features, NavigationAction(requestWithReferrer.resourceRequest()));
     3450    Page* page = oldPage->chrome().createWindow(&openerFrame, requestWithReferrer, features, NavigationAction(requestWithReferrer.resourceRequest()));
    34513451    if (!page)
    34523452        return nullptr;
    … …  
    34543454    RefPtr<Frame> frame = &page->mainFrame();
    34553455
    3456     frame->loader().forceSandboxFlags(openerFrame->document()->sandboxFlags());
     3456    frame->loader().forceSandboxFlags(openerFrame.document()->sandboxFlags());
    34573457
    34583458    if (request.frameName() != "_blank")

  • trunk/Source/WebCore/loader/FrameLoader.h

    r183698 r183781  
    459459// FIXME: Consider making this function part of an appropriate class (not FrameLoader)
    460460// and moving it to a more appropriate location.
    461 PassRefPtr<Frame> createWindow(Frame* openerFrame, Frame* lookupFrame, const FrameLoadRequest&, const WindowFeatures&, bool& created);
     461PassRefPtr<Frame> createWindow(Frame& openerFrame, Frame* lookupFrame, const FrameLoadRequest&, const WindowFeatures&, bool& created);
    462462
    463463} // namespace WebCore

  • trunk/Source/WebCore/page/DOMWindow.cpp

    r183498 r183781  
    20762076{
    20772077    Frame* activeFrame = activeWindow.frame();
     2078    if (!activeFrame)
     2079        return nullptr;
    20782080
    20792081    URL completedURL = urlString.isEmpty() ? URL(ParsedURLString, emptyString()) : firstFrame->document()->completeURL(urlString);
    … …  
    20812083        // Don't expose client code to invalid URLs.
    20822084        activeWindow.printErrorMessage("Unable to open a window with invalid URL '" + completedURL.string() + "'.\n");
    2083         return 0;
     2085        return nullptr;
    20842086    }
    20852087
    … …  
    20942096    // the opener frame, and the name references a frame relative to the opener frame.
    20952097    bool created;
    2096     RefPtr<Frame> newFrame = WebCore::createWindow(activeFrame, openerFrame, frameRequest, windowFeatures, created);
     2098    RefPtr<Frame> newFrame = WebCore::createWindow(*activeFrame, openerFrame, frameRequest, windowFeatures, created);
    20972099    if (!newFrame)
    2098         return 0;
     2100        return nullptr;
    20992101
    21002102    newFrame->loader().setOpener(openerFrame);
    … …  
    21182120    // Navigating the new frame could result in it being detached from its page by a navigation policy delegate.
    21192121    if (!newFrame->page())
    2120         return 0;
     2122        return nullptr;
    21212123
    21222124    return newFrame.release();
Note: See TracChangeset for help on using the changeset viewer.