Changeset 29380 in webkit


Ignore:
Timestamp:
Jan 10, 2008 4:23:13 PM (16 years ago)
Author:
weinig@apple.com
Message:

WebCore:

Reviewed by Sam Weinig and Anders Carlsson.

Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
<rdar://problem/5657355>

This patch makes two changes:

1) Java calls FrameLoader::load in a slightly different way than

JavaScript, which previously let a malicious web site bypass the
shouldAllowNavigation check. This patch adds that check to that
code path.

2) FrameLoader now wraps calls to m_frame->tree()->find(name) with

findFrameForNavigation, which calls shouldAllowNavigation. This
treats disallowed frame navigations as if the named frame did not
exist, resulting in a popup window when appropriate.

Tests: http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html

http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html

  • WebCore.base.exp:
  • bindings/js/kjs_window.cpp: (KJS::WindowProtoFuncOpen::callAsFunction):
  • loader/FrameLoader.cpp: (WebCore::FrameLoader::createWindow): (WebCore::FrameLoader::load): (WebCore::FrameLoader::post): (WebCore::FrameLoader::findFrameForNavigation):
  • loader/FrameLoader.h:

WebKit/mac:

Reviewed by Anders Carlsson.

Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
<rdar://problem/5657355>

  • Plugins/WebBaseNetscapePluginView.mm: (-[WebBaseNetscapePluginView loadPluginRequest:]): call findFrameForNavigation to ensure the shouldAllowNavigation check is made.

WebKitTools:

Reviewed by Anders Carlsson.

Make DRT track open windows instead of allocated windows so that
we can avoid ASSERTION due to late deallocs out of our control.

  • DumpRenderTree/mac/DumpRenderTree.mm: (dumpBackForwardListForAllWindows): (runTest):
  • DumpRenderTree/mac/DumpRenderTreeMac.h:
  • DumpRenderTree/mac/DumpRenderTreeWindow.h:
  • DumpRenderTree/mac/DumpRenderTreeWindow.mm: (+[DumpRenderTreeWindow openWindows]): (-[DumpRenderTreeWindow initWithContentRect:styleMask:backing:defer:]): (-[DumpRenderTreeWindow close]):
  • DumpRenderTree/mac/LayoutTestControllerMac.mm: (LayoutTestController::windowCount):

LayoutTests:

Reviewed by Anders Carlsson.

Tests for http://bugs.webkit.org/show_bug.cgi?id=16522
<rdar://problem/5657355>

  • http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html: Added.
  • http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html: Added.
  • http/tests/security/frameNavigation/resources/navigation-happened.html: Added.
  • http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt: Added.
  • http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html: Added.
  • http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt: Added.
  • http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html: Added.
  • platform/win/Skipped:
Location:
trunk
Files:
7 added
15 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r29374 r29380  
     12008-01-10  Sam Weinig  <sam@webkit.org>
     2
     3        Reviewed by Anders Carlsson.
     4
     5        Tests for http://bugs.webkit.org/show_bug.cgi?id=16522
     6        <rdar://problem/5657355>
     7
     8        * http/tests/security/frameNavigation/resources/frame-with-link-to-navigate.html: Added.
     9        * http/tests/security/frameNavigation/resources/frame-with-plugin-to-navigate.html: Added.
     10        * http/tests/security/frameNavigation/resources/navigation-happened.html: Added.
     11        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation-expected.txt: Added.
     12        * http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html: Added.
     13        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation-expected.txt: Added.
     14        * http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html: Added.
     15        * platform/win/Skipped:
     16
    1172008-01-10  Adam Roben  <aroben@apple.com>
    218
  • trunk/LayoutTests/platform/win/Skipped

    r29374 r29380  
    203203http/tests/security/aboutBlank/xss-DENIED-navigate-opener-document-write.html
    204204http/tests/security/aboutBlank/xss-DENIED-navigate-opener-javascript-url.html
     205http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
     206http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
    205207
    206208# DRT is not fully implemented in boomer <rdar://problem/5128261>
  • trunk/WebCore/ChangeLog

    r29379 r29380  
     12008-01-10  Adam Barth  <hk9565@gmail.com>
     2
     3        Reviewed by Sam Weinig and Anders Carlsson.
     4
     5        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
     6        <rdar://problem/5657355>
     7
     8        This patch makes two changes:
     9
     10        1) Java calls FrameLoader::load in a slightly different way than
     11           JavaScript, which previously let a malicious web site bypass the
     12           shouldAllowNavigation check.  This patch adds that check to that
     13           code path.
     14
     15        2) FrameLoader now wraps calls to m_frame->tree()->find(name) with
     16           findFrameForNavigation, which calls shouldAllowNavigation.  This
     17           treats disallowed frame navigations as if the named frame did not
     18           exist, resulting in a popup window when appropriate.
     19
     20        Tests: http/tests/security/frameNavigation/xss-DENIED-plugin-navigation.html
     21               http/tests/security/frameNavigation/xss-DENIED-targeted-link-navigation.html
     22
     23        * WebCore.base.exp:
     24        * bindings/js/kjs_window.cpp:
     25        (KJS::WindowProtoFuncOpen::callAsFunction):
     26        * loader/FrameLoader.cpp:
     27        (WebCore::FrameLoader::createWindow):
     28        (WebCore::FrameLoader::load):
     29        (WebCore::FrameLoader::post):
     30        (WebCore::FrameLoader::findFrameForNavigation):
     31        * loader/FrameLoader.h:
     32
    1332008-01-10  John Sullivan  <sullivan@apple.com>
    234
  • trunk/WebCore/WebCore.base.exp

    r29257 r29380  
    1 
    21.objc_class_name_DOMAbstractView
    32.objc_class_name_DOMAttr
     
    153152__ZN7WebCore11FrameLoader20continueLoadWithDataEPNS_12SharedBufferERKNS_6StringES5_RKNS_4KURLE
    154153__ZN7WebCore11FrameLoader21setCurrentHistoryItemEN3WTF10PassRefPtrINS_11HistoryItemEEE
     154__ZN7WebCore11FrameLoader22findFrameForNavigationERKNS_12AtomicStringE
    155155__ZN7WebCore11FrameLoader22setPreviousHistoryItemEN3WTF10PassRefPtrINS_11HistoryItemEEE
    156156__ZN7WebCore11FrameLoader23reloadAllowingStaleDataERKNS_6StringE
     
    733733_wkDrawCapsLockIndicator
    734734_wkDrawFocusRing
     735_wkDrawMediaFullscreenButton
     736_wkDrawMediaMuteButton
     737_wkDrawMediaPauseButton
     738_wkDrawMediaPlayButton
     739_wkDrawMediaSeekBackButton
     740_wkDrawMediaSeekForwardButton
     741_wkDrawMediaSliderThumb
     742_wkDrawMediaUnMuteButton
    735743_wkDrawTextFieldCellFocusRing
    736744_wkFontSmoothingModeIsLCD
     
    747755_wkGetMIMETypeForExtension
    748756_wkGetMediaControlBackgroundImageData
    749 _wkDrawMediaFullscreenButton
    750 _wkDrawMediaMuteButton
    751 _wkDrawMediaPauseButton
    752 _wkDrawMediaPlayButton
    753 _wkDrawMediaSeekBackButton
    754 _wkDrawMediaSeekForwardButton
    755 _wkDrawMediaSliderThumb
    756 _wkDrawMediaUnMuteButton
    757757_wkGetNSFontATSUFontId
    758758_wkGetNSURLResponseCalculatedExpiration
  • trunk/WebCore/bindings/js/kjs_window.cpp

    r29365 r29380  
    330330    FrameLoadRequest frameRequest(request, frameName);
    331331
     332    FrameLoader* loader;
     333    if (activeFrame)
     334        // We need to use the active frame's loader to let FrameLoader know
     335        // which principal is requesting the navigation.  Unfortunately, there
     336        // might not be an activeFrame, in which case we resort to using the
     337        // opener's loader.
     338        //
     339        // See http://bugs.webkit.org/show_bug.cgi?id=16522
     340        loader = activeFrame->loader();
     341    else
     342        loader = openerFrame->loader();
     343
    332344    // FIXME: It's much better for client API if a new window starts with a URL, here where we
    333345    // know what URL we are going to open. Unfortunately, this code passes the empty string
     
    338350
    339351    bool created;
    340     Frame* newFrame = openerFrame->loader()->createWindow(frameRequest, windowFeatures, created);
     352    Frame* newFrame = loader->createWindow(frameRequest, windowFeatures, created);
    341353    if (!newFrame)
    342354        return 0;
     
    10641076        if (Frame* parent = frame->tree()->parent())
    10651077            frame = parent;
     1078        topOrParent = true;
     1079    }
     1080    if (topOrParent) {
    10661081        if (!activeFrame->loader()->shouldAllowNavigation(frame))
    10671082            return jsUndefined();
    1068         topOrParent = true;
    1069     }
    1070     if (topOrParent) {
     1083
    10711084        String completedURL;
    10721085        if (!urlString.isEmpty())
  • trunk/WebCore/loader/FrameLoader.cpp

    r29352 r29380  
    310310
    311311    if (!request.frameName().isEmpty() && request.frameName() != "_blank")
    312         if (Frame* frame = m_frame->tree()->find(request.frameName())) {
    313             if (!shouldAllowNavigation(frame))
    314                 return 0;
     312        if (Frame* frame = findFrameForNavigation(request.frameName())) {
    315313            if (!request.resourceRequest().url().isEmpty())
    316314                frame->loader()->load(request, false, true, 0, 0, HashMap<String, String>());
     
    19491947        referrer = String();
    19501948   
    1951     Frame* targetFrame = m_frame->tree()->find(request.frameName());
    1952     if (!shouldAllowNavigation(targetFrame))
    1953         return;
     1949    Frame* targetFrame = findFrameForNavigation(request.frameName());
    19541950       
    19551951    if (request.resourceRequest().httpMethod() != "POST") {
     
    19941990
    19951991    if (!frameName.isEmpty()) {
    1996         if (Frame* targetFrame = m_frame->tree()->find(frameName))
     1992        if (Frame* targetFrame = findFrameForNavigation(frameName))
    19971993            targetFrame->loader()->load(newURL, referrer, newLoadType, String(), event, formState);
    19981994        else
     
    20662062    }
    20672063
    2068     Frame* frame = m_frame->tree()->find(frameName);
     2064    Frame* frame = findFrameForNavigation(frameName);
    20692065    if (frame) {
    20702066        frame->loader()->load(request);
     
    32453241
    32463242    if (!frameName.isEmpty()) {
    3247         if (Frame* targetFrame = m_frame->tree()->find(frameName))
     3243        if (Frame* targetFrame = findFrameForNavigation(frameName))
    32483244            targetFrame->loader()->load(request, action, FrameLoadTypeStandard, formState.release());
    32493245        else
     
    38703866        bfItem->setIsTargetItem(true);
    38713867    return bfItem;
     3868}
     3869
     3870Frame* FrameLoader::findFrameForNavigation(const AtomicString& name)
     3871{
     3872    Frame* frame = m_frame->tree()->find(name);
     3873    if (shouldAllowNavigation(frame))
     3874        return frame; 
     3875    return 0;
    38723876}
    38733877
  • trunk/WebCore/loader/FrameLoader.h

    r29322 r29380  
    436436
    437437        bool shouldAllowNavigation(Frame* targetFrame) const;
     438        Frame* findFrameForNavigation(const AtomicString& name);
    438439
    439440    private:
  • trunk/WebKit/mac/ChangeLog

    r29268 r29380  
     12008-01-10  Sam Weinig  <sam@webkit.org>
     2
     3        Reviewed by Anders Carlsson.
     4
     5        Fixes: http://bugs.webkit.org/show_bug.cgi?id=16522
     6        <rdar://problem/5657355>
     7
     8        * Plugins/WebBaseNetscapePluginView.mm:
     9        (-[WebBaseNetscapePluginView loadPluginRequest:]): call findFrameForNavigation
     10        to ensure the shouldAllowNavigation check is made.
     11
    1122008-01-07  Nikolas Zimmermann  <zimmermann@kde.org>
    213
  • trunk/WebKit/mac/Plugins/WebBaseNetscapePluginView.mm

    r29126 r29380  
    21312131        // FIXME - need to get rid of this window creation which
    21322132        // bypasses normal targeted link handling
    2133         frame = [[self webFrame] findFrameNamed:frameName];
    2134    
     2133        frame = kit([[self webFrame] _frameLoader]->findFrameForNavigation(frameName));
    21352134        if (frame == nil) {
    21362135            WebView *currentWebView = [self webView];
  • trunk/WebKitTools/ChangeLog

    r29376 r29380  
     12008-01-10  Sam Weinig  <sam@webkit.org>
     2
     3        Reviewed by Anders Carlsson.
     4
     5        Make DRT track open windows instead of allocated windows so that
     6        we can avoid ASSERTION due to late deallocs out of our control.
     7
     8        * DumpRenderTree/mac/DumpRenderTree.mm:
     9        (dumpBackForwardListForAllWindows):
     10        (runTest):
     11        * DumpRenderTree/mac/DumpRenderTreeMac.h:
     12        * DumpRenderTree/mac/DumpRenderTreeWindow.h:
     13        * DumpRenderTree/mac/DumpRenderTreeWindow.mm:
     14        (+[DumpRenderTreeWindow openWindows]):
     15        (-[DumpRenderTreeWindow initWithContentRect:styleMask:backing:defer:]):
     16        (-[DumpRenderTreeWindow close]):
     17        * DumpRenderTree/mac/LayoutTestControllerMac.mm:
     18        (LayoutTestController::windowCount):
     19
    1202008-01-10  Ada Chan  <adachan@apple.com>
    221
  • trunk/WebKitTools/DumpRenderTree/mac/DumpRenderTree.mm

    r29365 r29380  
    707707static void dumpBackForwardListForAllWindows()
    708708{
    709     CFArrayRef allWindows = (CFArrayRef)[DumpRenderTreeWindow allWindows];
    710     unsigned count = CFArrayGetCount(allWindows);
     709    CFArrayRef openWindows = (CFArrayRef)[DumpRenderTreeWindow openWindows];
     710    unsigned count = CFArrayGetCount(openWindows);
    711711    for (unsigned i = 0; i < count; i++) {
    712         NSWindow *window = (NSWindow *)CFArrayGetValueAtIndex(allWindows, i);
     712        NSWindow *window = (NSWindow *)CFArrayGetValueAtIndex(openWindows, i);
    713713        WebView *webView = [[[window contentView] subviews] objectAtIndex:0];
    714714        dumpBackForwardListForWebView(webView);
     
    867867
    868868    if (layoutTestController->closeRemainingWindowsWhenComplete()) {
    869         NSArray* array = [DumpRenderTreeWindow allWindows];
    870        
     869        NSArray* array = [DumpRenderTreeWindow openWindows];
     870
    871871        unsigned count = [array count];
    872872        for (unsigned i = 0; i < count; i++) {
     
    889889    [pool release];
    890890
    891     // We should only have our main window left when we're done
    892     ASSERT(CFArrayGetCount(allWindowsRef) == 1);
    893     ASSERT(CFArrayGetValueAtIndex(allWindowsRef, 0) == [[mainFrame webView] window]);
     891    // We should only have our main window left open when we're done
     892    ASSERT(CFArrayGetCount(openWindowsRef) == 1);
     893    ASSERT(CFArrayGetValueAtIndex(openWindowsRef, 0) == [[mainFrame webView] window]);
    894894
    895895    delete layoutTestController;
  • trunk/WebKitTools/DumpRenderTree/mac/DumpRenderTreeMac.h

    r28419 r29380  
    3939@class WebView;
    4040
    41 extern CFMutableArrayRef allWindowsRef;
     41extern CFMutableArrayRef openWindowsRef;
    4242extern CFMutableSetRef disallowedURLs;
    4343extern WebFrame* mainFrame;
  • trunk/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.h

    r27949 r29380  
    3333@interface DumpRenderTreeWindow : NSWindow
    3434// I'm not sure why we can't just use [NSApp windows]
    35 + (NSArray *)allWindows;
     35+ (NSArray *)openWindows;
    3636@end
  • trunk/WebKitTools/DumpRenderTree/mac/DumpRenderTreeWindow.mm

    r27949 r29380  
    3636#import "LayoutTestController.h"
    3737
    38 CFMutableArrayRef allWindowsRef = 0;
     38CFMutableArrayRef openWindowsRef = 0;
    3939
    4040static CFArrayCallBacks NonRetainingArrayCallbacks = {
     
    4848@implementation DumpRenderTreeWindow
    4949
    50 + (NSArray *)allWindows
     50+ (NSArray *)openWindows
    5151{
    52     return [[(NSArray *)allWindowsRef copy] autorelease];
     52    return [[(NSArray *)openWindowsRef copy] autorelease];
    5353}
    5454
    5555- (id)initWithContentRect:(NSRect)contentRect styleMask:(unsigned int)styleMask backing:(NSBackingStoreType)bufferingType defer:(BOOL)deferCreation
    5656{
    57     if (!allWindowsRef)
    58         allWindowsRef = CFArrayCreateMutable(NULL, 0, &NonRetainingArrayCallbacks);
     57    if (!openWindowsRef)
     58        openWindowsRef = CFArrayCreateMutable(NULL, 0, &NonRetainingArrayCallbacks);
    5959
    60     CFArrayAppendValue(allWindowsRef, self);
     60    CFArrayAppendValue(openWindowsRef, self);
    6161           
    6262    return [super initWithContentRect:contentRect styleMask:styleMask backing:bufferingType defer:deferCreation];
    6363}
    6464
    65 - (void)dealloc
     65- (void)close
    6666{
    67     CFRange arrayRange = CFRangeMake(0, CFArrayGetCount(allWindowsRef));
    68     CFIndex i = CFArrayGetFirstIndexOfValue(allWindowsRef, arrayRange, self);
     67    CFRange arrayRange = CFRangeMake(0, CFArrayGetCount(openWindowsRef));
     68    CFIndex i = CFArrayGetFirstIndexOfValue(openWindowsRef, arrayRange, self);
    6969    assert(i != -1);
    70 
    71     CFArrayRemoveValueAtIndex(allWindowsRef, i);
    72     [super dealloc];
     70    CFArrayRemoveValueAtIndex(openWindowsRef, i);
     71    [super close];
    7372}
    7473
  • trunk/WebKitTools/DumpRenderTree/mac/LayoutTestControllerMac.mm

    r29365 r29380  
    241241int LayoutTestController::windowCount()
    242242{
    243     return CFArrayGetCount(allWindowsRef);
     243    return CFArrayGetCount(openWindowsRef);
    244244}
    245245
Note: See TracChangeset for help on using the changeset viewer.