Changeset 28056 in webkit


Ignore:
Timestamp:
Nov 26, 2007 4:15:21 PM (16 years ago)
Author:
weinig@apple.com
Message:

WebCore:

Reviewed by Darin.

Fix for <rdar://problem/5592988>

  • Enforce tighter restrictions on what frames in other domains can be navigated.

Tests: http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html

http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html

  • bindings/js/kjs_window.cpp: (KJS::Window::put): (KJS::Location::put): (KJS::LocationProtoFuncReplace::callAsFunction): (KJS::LocationProtoFuncAssign::callAsFunction):
  • loader/FrameLoader.cpp: (WebCore::FrameLoader::createWindow): (WebCore::FrameLoader::load): (WebCore::FrameLoader::shouldAllowNavigation): Move and update logic from canTarget().
  • loader/FrameLoader.h:
  • page/FrameTree.cpp: (WebCore::FrameTree::isDescendantOf): Make this O(1) in the case when both frames are not in the same page.

LayoutTests:

Reviewed by Darin.

Tests for <rdar://problem/5592988>

  • Update and add tests for new tighter restrictions on what frames in other domains can be navigated.
  • http/tests/security/cross-frame-access-location-expected.txt:
  • http/tests/security/frameNavigation: Added.
  • http/tests/security/frameNavigation/resources: Added.
  • http/tests/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html: Added.
  • http/tests/security/frameNavigation/resources/iframe-with-inner-frame-on-foreign-domain.html: Added.
  • http/tests/security/frameNavigation/resources/navigation-changed-iframe.html: Added.
  • http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-expected.txt: Added.
  • http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html: Added.
  • http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change-expected.txt: Added.
  • http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html: Added.
Location:
trunk
Files:
9 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r28048 r28056  
     12007-11-26  Sam Weinig  <sam@webkit.org>
     2
     3        Reviewed by Darin.
     4
     5        Tests for <rdar://problem/5592988>
     6
     7        - Update and add tests for new tighter restrictions on what frames in other domains
     8          can be navigated.
     9
     10        * http/tests/security/cross-frame-access-location-expected.txt:
     11        * http/tests/security/frameNavigation: Added.
     12        * http/tests/security/frameNavigation/resources: Added.
     13        * http/tests/security/frameNavigation/resources/iframe-that-performs-parent-navigation.html: Added.
     14        * http/tests/security/frameNavigation/resources/iframe-with-inner-frame-on-foreign-domain.html: Added.
     15        * http/tests/security/frameNavigation/resources/navigation-changed-iframe.html: Added.
     16        * http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change-expected.txt: Added.
     17        * http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html: Added.
     18        * http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change-expected.txt: Added.
     19        * http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html: Added.
     20
    1212007-11-26  Dan Bernstein  <mitz@apple.com>
    222
  • trunk/LayoutTests/http/tests/security/cross-frame-access-location-expected.txt

    r25028 r28056  
    1 CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-get-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-location.html. Domains, protocols and ports must match.
    2 
    31CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://localhost:8000/security/resources/cross-frame-iframe-for-get-test.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-location.html. Domains, protocols and ports must match.
    42
  • trunk/WebCore/ChangeLog

    r28055 r28056  
     12007-11-26  Sam Weinig  <sam@webkit.org>
     2
     3        Reviewed by Darin.
     4
     5        Fix for <rdar://problem/5592988>
     6        - Enforce tighter restrictions on what frames in other domains
     7          can be navigated.
     8
     9        Tests: http/tests/security/frameNavigation/xss-ALLOWED-parent-navigation-change.html
     10               http/tests/security/frameNavigation/xss-ALLOWED-targeted-subframe-navigation-change.html
     11
     12        * bindings/js/kjs_window.cpp:
     13        (KJS::Window::put):
     14        (KJS::Location::put):
     15        (KJS::LocationProtoFuncReplace::callAsFunction):
     16        (KJS::LocationProtoFuncAssign::callAsFunction):
     17        * loader/FrameLoader.cpp:
     18        (WebCore::FrameLoader::createWindow):
     19        (WebCore::FrameLoader::load):
     20        (WebCore::FrameLoader::shouldAllowNavigation): Move and update logic from canTarget().
     21        * loader/FrameLoader.h:
     22        * page/FrameTree.cpp:
     23        (WebCore::FrameTree::isDescendantOf): Make this O(1) in the case when both frames are not
     24        in the same page.
     25
    1262007-11-26  Steve Falkenburg  <sfalken@apple.com>
    227
  • trunk/WebCore/bindings/js/kjs_window.cpp

    r27885 r28056  
    732732      Frame* p = Window::retrieveActive(exec)->impl()->frame();
    733733      if (p) {
     734        if (!p->loader()->shouldAllowNavigation(impl()->frame()))
     735          return;
    734736        DeprecatedString dstUrl = p->loader()->completeURL(DeprecatedString(value->toString(exec))).url();
    735737        if (!dstUrl.startsWith("javascript:", false) || isSafeScript(exec)) {
     
    19611963      case Href: {
    19621964          Frame* frame = Window::retrieveActive(exec)->impl()->frame();
    1963           if (frame)
    1964               url = frame->loader()->completeURL(str).url();
    1965           else
    1966               url = str;
     1965          if (!frame)
     1966              return;
     1967          if (!frame->loader()->shouldAllowNavigation(m_frame))
     1968              return;
     1969          url = frame->loader()->completeURL(str).url();
    19671970          break;
    19681971      }
     
    20232026        return jsUndefined();
    20242027
    2025     DeprecatedString str = args[0]->toString(exec);
    20262028    Frame* p = Window::retrieveActive(exec)->impl()->frame();
    2027     if ( p ) {
     2029    if (p) {
     2030      if (!p->loader()->shouldAllowNavigation(frame))
     2031        return jsUndefined();
     2032      DeprecatedString str = args[0]->toString(exec);
    20282033      const Window* window = Window::retrieveWindow(frame);
    20292034      if (!str.startsWith("javascript:", false) || (window && window->isSafeScript(exec))) {
     
    20652070        return jsUndefined();
    20662071
    2067     Window* window = Window::retrieveWindow(frame);
    2068     if (!window->isSafeScript(exec))
    2069         return jsUndefined();
    2070 
    20712072    Frame *p = Window::retrieveActive(exec)->impl()->frame();
    20722073    if (p) {
     2074        if (!p->loader()->shouldAllowNavigation(frame))
     2075          return jsUndefined();
    20732076        const Window *window = Window::retrieveWindow(frame);
    20742077        DeprecatedString dstUrl = p->loader()->completeURL(DeprecatedString(args[0]->toString(exec))).url();
  • trunk/WebCore/loader/FrameLoader.cpp

    r27986 r28056  
    307307    if (!request.frameName().isEmpty() && request.frameName() != "_blank")
    308308        if (Frame* frame = m_frame->tree()->find(request.frameName())) {
     309            if (!shouldAllowNavigation(frame))
     310                return 0;
    309311            if (!request.resourceRequest().url().isEmpty())
    310312                frame->loader()->load(request, false, true, 0, 0, HashMap<String, String>());
     
    19361938   
    19371939    Frame* targetFrame = m_frame->tree()->find(request.frameName());
    1938     if (!canTarget(targetFrame))
     1940    if (!shouldAllowNavigation(targetFrame))
    19391941        return;
    19401942       
     
    23052307}
    23062308
    2307 bool FrameLoader::canTarget(Frame* target) const
    2308 {
    2309     // This function prevents this exploit:
     2309bool FrameLoader::shouldAllowNavigation(Frame* targetFrame) const
     2310{
     2311    // This function prevents these exploits:
    23102312    // <rdar://problem/3715785> multiple frame injection vulnerability reported by Secunia, affects almost all browsers
     2313    // http://bugs.webkit.org/show_bug.cgi?id=15936 Overly permissive frame navigation allows password theft
    23112314
    23122315    // Allow if there is no specific target.
    2313     if (!target)
     2316    if (!targetFrame)
    23142317        return true;
    2315 
    2316     // Allow navigation within the same page/frameset.
    2317     if (m_frame->page() == target->page())
     2318    if (m_frame == targetFrame)
    23182319        return true;
    23192320
    2320     // Allow if the request is made from a local file.
    2321     ASSERT(m_frame->document());
    2322     String domain = m_frame->document()->domain();
    2323     if (domain.isEmpty())
     2321    // The navigation change is safe if the active frame is:
     2322    //   - in the same security domain (satisfies same-origin policy)
     2323    //   - the opener frame
     2324    //   - an ancestor or a descendant in frame tree hierarchy
     2325
     2326    // Same security domain case.
     2327    Document* activeDocument = m_frame->document();
     2328    ASSERT(activeDocument);
     2329    Document* targetDocument = targetFrame->document();
     2330    if (!targetDocument)
    23242331        return true;
    2325    
    2326     // Allow if target is an entire window (top level frame of a window).
    2327     Frame* parent = target->tree()->parent();
    2328     if (!parent)
     2332    const SecurityOrigin& activeSecurityOrigin = activeDocument->securityOrigin();
     2333    const SecurityOrigin& targetSecurityOrigin = targetDocument->securityOrigin();
     2334    if (activeSecurityOrigin.canAccess(targetSecurityOrigin))
    23292335        return true;
    2330    
    2331     // Allow if the domain of the parent of the targeted frame equals this domain.
    2332     String parentDomain;
    2333     if (Document* parentDocument = parent->document())
    2334         parentDomain = parentDocument->domain();
    2335     return equalIgnoringCase(parentDomain, domain);
     2336
     2337    // Opener case.
     2338    if (m_frame == targetFrame->loader()->opener())
     2339        return true;
     2340
     2341    // Ancestor or descendant case.
     2342    if (targetFrame->tree()->isDescendantOf(m_frame) || m_frame->tree()->isDescendantOf(targetFrame))
     2343        return true;
     2344
     2345    if (!targetFrame->settings()->privateBrowsingEnabled()) {
     2346        // FIXME: this error message should contain more specifics of why the navigation change is not allowed.
     2347        String message = String::format("Unsafe JavaScript attempt to initiate a navigation change for frame with URL %s from frame with URL %s.\n",
     2348                                        targetDocument->URL().utf8().data(), activeDocument->URL().utf8().data());
     2349
     2350        if (KJS::Interpreter::shouldPrintExceptions())
     2351            printf("%s", message.utf8().data());
     2352
     2353        // FIXME: should we print to the console of the activeFrame as well?
     2354        if (Page* page = targetFrame->page())
     2355            page->chrome()->addMessageToConsole(JSMessageSource, ErrorMessageLevel, message, 1, String());
     2356    }
     2357   
     2358    return false;
    23362359}
    23372360
  • trunk/WebCore/loader/FrameLoader.h

    r27729 r28056  
    437437
    438438        void iconLoadDecisionAvailable();
     439
     440        bool shouldAllowNavigation(Frame* targetFrame) const;
     441
    439442    private:
    440443        PassRefPtr<HistoryItem> createHistoryItem(bool useOriginal);
     
    537540        void applyUserAgent(ResourceRequest& request);
    538541
    539         bool canTarget(Frame*) const;
    540 
    541542        void scheduleRedirection(ScheduledRedirection*);
    542543        void startRedirectionTimer();
  • trunk/WebCore/page/FrameTree.cpp

    r26589 r28056  
    201201bool FrameTree::isDescendantOf(const Frame* ancestor) const
    202202{
     203    if (m_thisFrame->page() != ancestor->page())
     204        return false;
     205
    203206    for (Frame* frame = m_thisFrame; frame; frame = frame->tree()->parent())
    204207        if (frame == ancestor)
Note: See TracChangeset for help on using the changeset viewer.