Changeset 28912 in webkit
- Timestamp:
- Dec 20, 2007 2:39:51 PM (16 years ago)
- Location:
- trunk
- Files:
-
- 4 added
- 16 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r28908 r28912 1 2007-12-20 Adam Barth <hk9565@gmail.com> 2 3 Reviewed and landed by Sam Weinig. 4 5 Update LayoutTests for http://bugs.webkit.org/show_bug.cgi?id=15313 6 7 * http/tests/security/cross-frame-access-child-explicit-domain-expected.txt: Added. 8 * http/tests/security/cross-frame-access-child-explicit-domain.html: Added. 9 * http/tests/security/cross-frame-access-custom-expected.txt: 10 * http/tests/security/cross-frame-access-parent-explicit-domain-expected.txt: Added. 11 * http/tests/security/cross-frame-access-parent-explicit-domain.html: Added. 12 * http/tests/security/cross-frame-access-port-explicit-domain-expected.txt: 13 * http/tests/security/cross-frame-access-protocol-explicit-domain-expected.txt: 14 * http/tests/security/cross-frame-access-protocol-explicit-domain.html: 15 1 16 2007-12-20 johnnyding.webkit <johnnyding.webkit@gmail.com> 2 17 -
trunk/LayoutTests/http/tests/security/cross-frame-access-custom-expected.txt
r24346 r28912 1 CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe-with-explicit-domain-set.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-child-explicit-domain.html. Domains, protocols and ports must match. 2 1 3 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-custom.html. Domains, protocols and ports must match. 2 4 -
trunk/LayoutTests/http/tests/security/cross-frame-access-port-explicit-domain-expected.txt
r24863 r28912 1 CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL http://127.0.0.1:8000/security/resources/cross-frame-iframe.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-parent-explicit-domain.html. Domains, protocols and ports must match. 2 1 3 This test currently fails because we check the port and protocol even if document.domain is explicitly set (rdar://problem/5366437). 2 4 -
trunk/LayoutTests/http/tests/security/cross-frame-access-protocol-explicit-domain-expected.txt
r24863 r28912 1 CONSOLE MESSAGE: line 1: Unsafe JavaScript attempt to access frame with URL https://127.0.0.1:8443/security/resources/cross-frame-iframe-with-explicit-domain-set.html from frame with URL http://127.0.0.1:8000/security/cross-frame-access-protocol-explicit-domain.html. Domains, protocols and ports must match. 2 1 3 This test currently fails because we check the port and protocol even if document.domain is explicitly set (rdar://problem/5366437). 2 4 3 5 4 PASS: Cross frame access to https from http, after explicitly setting document.domain, was allowed!6 PASS: Cross frame access to https from http, after explicitly setting document.domain, was denied. 5 7 6 8 … … 8 10 Frame: 'aFrame' 9 11 -------- 10 PASS: Cross frame access to https from http, after explicitly setting document.domain, was allowed!11 12 12 Inner iframe with explicit document.domain set. -
trunk/LayoutTests/http/tests/security/cross-frame-access-protocol-explicit-domain.html
r24781 r28912 13 13 var url = "https://127.0.0.1:8443/security/resources/cross-frame-iframe-with-explicit-domain-set.html"; 14 14 var iframeId ="aFrame"; 15 var passMessage = "PASS: Cross frame access to https from http, after explicitly setting document.domain, was allowed!";16 var failMessage = "Fail: Cross frame access to https from http, after explicitly setting document.domain, was denied.";17 can AccessFrame(url, iframeId, passMessage, failMessage);15 var passMessage = "PASS: Cross frame access to https from http, after explicitly setting document.domain, was denied."; 16 var failMessage = "Fail: Cross frame access to https from http, after explicitly setting document.domain, was allowed."; 17 cannotAccessFrame(url, iframeId, passMessage, failMessage); 18 18 </script> 19 19 </body> -
trunk/WebCore/ChangeLog
r28911 r28912 1 2007-12-20 Adam Barth <hk9565@gmail.com> 2 3 Reviewed and landed by Sam Weinig. 4 5 http://bugs.webkit.org/show_bug.cgi?id=15313 6 <rdar://problem/5514516> 7 8 The same-origin check was incorrect in two cases (both fixed in this 9 patch): 10 11 A) If both the source and the target have set their document.domain 12 to the same value, the protocol must also match in order for 13 access to be allowed. Without this requirement, the browser is 14 vulnerable to the following attack: 15 16 1) Suppose there is an HTTPS site (www.example.com) that sets 17 document.domain = "example.com". 18 2) A network attacker redirects the browser to http://www.example.com/ 19 a) injects script to set document.domain = "example.com", and 20 b) opens a window to https://www.example.com/ 21 3) Now the network attacker can inject script into the HTTPS page, 22 stealing cookies and issuing banking transactions. 23 24 B) If only one of the source and target has set document.domain, then 25 access should be denied. With this behavior, the browser is 26 vulnerable to the following attack: 27 28 1) Suppose http://foo.example.com/ opens an iframe to 29 http://foo.example.com/frame.html that 30 a) sets document.domain = "example.com", and 31 b) opens an iframe to http://bar.example.com/ 32 This is a common usage of document.domain for cross-domain 33 communication, see for example: 34 http://www.collinjackson.com/research/papers/fp801-jackson.pdf 35 2) The inner-most iframe, which is from bar.example.com, sets 36 document.domain = "example.com". 37 3) Now the inner-most iframe can inject script into the middle 38 iframe (say via document.write). This bar.example.com script 39 now has access to the outer-most frame (from foo.example.com). 40 41 Both these changes cause WebKit to match the behavior of Firefox 2 and 42 IE6 in these cases. This patch includes regression tests for both 43 issues. 44 45 Internet Explorer 7 and Opera 9 are more strict in that they require 46 the port numbers to match when both pages have document.domain set. 47 Opera 9 allows access when only one page has set document.domain, but 48 this is a security vulnerability. 49 50 Tests: http/tests/security/cross-frame-access-child-explicit-domain.html 51 http/tests/security/cross-frame-access-parent-explicit-domain.html 52 53 * bindings/js/kjs_window.cpp: 54 (KJS::createWindow): 55 (KJS::Window::allowsAccessFrom): 56 * dom/Document.cpp: 57 (WebCore::Document::domain): 58 (WebCore::Document::setDomain): 59 (WebCore::Document::initSecurityOrigin): 60 * dom/Document.h: 61 (WebCore::Document::securityOrigin): 62 * loader/FrameLoader.cpp: 63 (WebCore::FrameLoader::begin): 64 (WebCore::FrameLoader::checkCallImplicitClose): 65 (WebCore::FrameLoader::shouldAllowNavigation): 66 * platform/SecurityOrigin.cpp: 67 (WebCore::SecurityOrigin::setForURL): 68 (WebCore::SecurityOrigin::createForFrame): 69 (WebCore::SecurityOrigin::canAccess): 70 * platform/SecurityOrigin.h: 71 (WebCore::SecurityOrigin::domain): 72 * storage/Database.cpp: 73 (WebCore::Database::openDatabase): 74 (WebCore::Database::Database): 75 (WebCore::Database::securityOriginData): 76 * storage/Database.h: 77 (WebCore::Database::databaseDebugName): 78 * storage/DatabaseTracker.cpp: 79 (WebCore::DatabaseTracker::canEstablishDatabase): 80 * storage/SQLTransaction.cpp: 81 (WebCore::SQLTransaction::postflightAndCommit): 82 (WebCore::SQLTransaction::cleanupAfterTransactionErrorCallback): 83 1 84 2007-12-20 Rodney Dawes <dobey@wayofthemonkey.com> 2 85 -
trunk/WebCore/bindings/js/kjs_window.cpp
r28884 r28912 356 356 if (created) { 357 357 newFrame->loader()->changeLocation(KURL(completedURL.deprecatedString()), activeFrame->loader()->outgoingReferrer(), false, userGesture); 358 if (Document* oldDoc = openerFrame->document()) { 359 newFrame->document()->setDomainInternal(oldDoc->domain()); 358 if (Document* oldDoc = openerFrame->document()) 360 359 newFrame->document()->setBaseURL(oldDoc->baseURL()); 361 }362 360 } else if (!url.isEmpty()) 363 361 newFrame->loader()->scheduleLocationChange(completedURL, activeFrame->loader()->outgoingReferrer(), false, userGesture); … … 820 818 WebCore::Document* originDocument = originFrame->document(); 821 819 822 const SecurityOrigin &originSecurityOrigin = originDocument->securityOrigin();823 const SecurityOrigin &targetSecurityOrigin = targetDocument->securityOrigin();824 825 if (originSecurityOrigin .canAccess(targetSecurityOrigin))820 const SecurityOrigin* originSecurityOrigin = originDocument->securityOrigin(); 821 const SecurityOrigin* targetSecurityOrigin = targetDocument->securityOrigin(); 822 823 if (originSecurityOrigin->canAccess(targetSecurityOrigin)) 826 824 return true; 827 825 -
trunk/WebCore/dom/Document.cpp
r28875 r28912 2595 2595 String Document::domain() const 2596 2596 { 2597 if (m_domain.isEmpty()) // not set yet (we set it on demand to save time and space) 2598 m_domain = KURL(url()).host(); // Initially set to the host 2599 return m_domain; 2597 return m_securityOrigin->domain(); 2600 2598 } 2601 2599 2602 2600 void Document::setDomain(const String& newDomain) 2603 2601 { 2604 // Not set yet (we set it on demand to save time and space)2605 // Initially set to the host2606 if (m_domain.isEmpty())2607 m_domain = KURL(url()).host();2608 2609 2602 // Both NS and IE specify that changing the domain is only allowed when 2610 2603 // the new domain is a suffix of the old domain. … … 2618 2611 // allow other pages loaded on different ports in the same domain that 2619 2612 // have also assigned to access this page. 2620 if (equalIgnoringCase( m_domain, newDomain)) {2621 m_securityOrigin .setDomainFromDOM(newDomain);2622 return; 2623 } 2624 2625 int oldLength = m_domain.length();2613 if (equalIgnoringCase(domain(), newDomain)) { 2614 m_securityOrigin->setDomainFromDOM(newDomain); 2615 return; 2616 } 2617 2618 int oldLength = domain().length(); 2626 2619 int newLength = newDomain.length(); 2627 // e.g. newDomain = webkit.org (10) and m_domain= www.webkit.org (14)2620 // e.g. newDomain = webkit.org (10) and domain() = www.webkit.org (14) 2628 2621 if (newLength >= oldLength) 2629 2622 return; 2630 2623 2631 String test = m_domain.copy();2624 String test = domain().copy(); 2632 2625 // Check that it's a subdomain, not e.g. "ebkit.org" 2633 2626 if (test[oldLength - newLength - 1] != '.') 2634 2627 return; 2635 2628 2636 // Now test is "webkit.org" from m_domain2629 // Now test is "webkit.org" from domain() 2637 2630 // and we check that it's the same thing as newDomain 2638 2631 test.remove(0, oldLength - newLength); … … 2640 2633 return; 2641 2634 2642 m_domain = newDomain; 2643 m_securityOrigin.setDomainFromDOM(newDomain); 2644 } 2645 2646 void Document::setDomainInternal(const String& newDomain) 2647 { 2648 m_domain = newDomain; 2635 m_securityOrigin->setDomainFromDOM(newDomain); 2649 2636 } 2650 2637 … … 3727 3714 void Document::initSecurityOrigin() 3728 3715 { 3729 if (!m_frame) 3730 return; 3731 m_securityOrigin.setForFrame(m_frame); 3716 m_securityOrigin = SecurityOrigin::createForFrame(m_frame); 3732 3717 } 3733 3718 -
trunk/WebCore/dom/Document.h
r28875 r28912 554 554 String domain() const; 555 555 void setDomain(const String& newDomain); 556 void setDomainInternal(const String& newDomain);557 556 558 557 String lastModified() const; … … 849 848 850 849 void initSecurityOrigin(); 851 const SecurityOrigin& securityOrigin() const { return m_securityOrigin; }850 SecurityOrigin* securityOrigin() const { return m_securityOrigin.get(); } 852 851 853 852 bool processingLoadEvent() const { return m_processingLoadEvent; } … … 867 866 void updateFocusAppearanceTimerFired(Timer<Document>*); 868 867 869 mutable String m_domain; 870 871 SecurityOrigin m_securityOrigin; 868 RefPtr<SecurityOrigin> m_securityOrigin; 872 869 873 870 RenderObject* m_savedRenderer; -
trunk/WebCore/loader/FrameLoader.cpp
r28875 r28912 888 888 void FrameLoader::begin(const KURL& url, bool dispatch) 889 889 { 890 bool resetScripting = !(m_isDisplayingInitialEmptyDocument && m_frame->document() && m_frame->document()->securityOrigin() .isSecureTransitionTo(url));890 bool resetScripting = !(m_isDisplayingInitialEmptyDocument && m_frame->document() && m_frame->document()->securityOrigin()->isSecureTransitionTo(url)); 891 891 clear(resetScripting, resetScripting); 892 892 if (dispatch) … … 1303 1303 return; 1304 1304 1305 // All frames completed -> set their domain to the frameset's domain1306 // This must only be done when loading the frameset initially (#22039),1307 // not when following a link in a frame (#44162).1308 if (m_frame->document()) {1309 String domain = m_frame->document()->domain();1310 for (Frame* child = m_frame->tree()->firstChild(); child; child = child->tree()->nextSibling())1311 if (child->document())1312 child->document()->setDomainInternal(domain);1313 }1314 1315 1305 m_didCallImplicitClose = true; 1316 1306 m_wasUnloadEventEmitted = false; … … 2352 2342 Document* activeDocument = m_frame->document(); 2353 2343 ASSERT(activeDocument); 2354 const SecurityOrigin &activeSecurityOrigin = activeDocument->securityOrigin();2344 const SecurityOrigin* activeSecurityOrigin = activeDocument->securityOrigin(); 2355 2345 for (Frame* ancestorFrame = targetFrame; ancestorFrame; ancestorFrame = ancestorFrame->tree()->parent()) { 2356 2346 Document* ancestorDocument = ancestorFrame->document(); … … 2358 2348 return true; 2359 2349 2360 const SecurityOrigin &ancestorSecurityOrigin = ancestorDocument->securityOrigin();2361 if (activeSecurityOrigin .canAccess(ancestorSecurityOrigin))2350 const SecurityOrigin* ancestorSecurityOrigin = ancestorDocument->securityOrigin(); 2351 if (activeSecurityOrigin->canAccess(ancestorSecurityOrigin)) 2362 2352 return true; 2363 2353 } -
trunk/WebCore/platform/SecurityOrigin.cpp
r28343 r28912 63 63 } 64 64 65 void SecurityOrigin::setFor Frame(Frame* frame)65 void SecurityOrigin::setForURL(const KURL& url) 66 66 { 67 67 clear(); 68 69 if (url.isEmpty()) 70 return; 71 72 m_protocol = url.protocol().lower(); 73 m_host = url.host().lower(); 74 m_port = url.port(); 75 76 if (m_port) 77 m_portSet = true; 78 79 // data: URLs are not allowed access to anything other than themselves. 80 if (m_protocol == "data") 81 m_noAccess = true; 82 } 83 84 PassRefPtr<SecurityOrigin> SecurityOrigin::createForFrame(Frame* frame) 85 { 86 RefPtr<SecurityOrigin> origin = new SecurityOrigin(); 87 88 if (!frame) 89 return origin; 68 90 69 91 FrameLoader* loader = frame->loader(); 70 92 const KURL& securityPolicyURL = loader->url(); 71 93 72 if (!securityPolicyURL.isEmpty()) { 73 m_protocol = securityPolicyURL.protocol().lower(); 74 m_host = securityPolicyURL.host().lower(); 75 m_port = securityPolicyURL.port(); 76 if (m_port) 77 m_portSet = true; 94 origin->setForURL(securityPolicyURL); 78 95 79 // data: URLs are not allowed access to anything other than themselves. 80 if (m_protocol == "data") { 81 m_noAccess = true; 82 return; 83 } 96 if (!origin->isEmpty() && origin->m_protocol != "about") 97 return origin; 84 98 85 // Only in the case of about:blank or javascript: URLs (which create documents using the "about" 86 // protocol) do we want to use the parent or openers URL as the origin. 87 if (m_protocol != "about") 88 return; 89 } 99 // In the case of about:blank or javascript: URLs (which create 100 // documents using the "about" protocol) do we want to use the 101 // parent or openers origin. 90 102 91 103 Frame* openerFrame = frame->tree()->parent(); … … 93 105 openerFrame = loader->opener(); 94 106 if (!openerFrame) 95 return ;107 return origin; 96 108 } 97 109 98 110 Document* openerDocument = openerFrame->document(); 99 111 if (!openerDocument) 100 return ;112 return origin; 101 113 102 *this = openerDocument->securityOrigin(); 114 // We alias the SecurityOrigins to match Firefox, see Bug 15313 115 // http://bugs.webkit.org/show_bug.cgi?id=15313 116 return openerDocument->securityOrigin(); 103 117 } 104 118 … … 109 123 } 110 124 111 bool SecurityOrigin::canAccess(const SecurityOrigin &other) const125 bool SecurityOrigin::canAccess(const SecurityOrigin* other) const 112 126 { 113 127 if (FrameLoader::shouldTreatSchemeAsLocal(m_protocol)) 114 128 return true; 115 129 116 if (m_noAccess || other .m_noAccess)130 if (m_noAccess || other->m_noAccess) 117 131 return false; 118 132 119 if (m_domainWasSetInDOM && other.m_domainWasSetInDOM && m_host == other.m_host) 120 return true; 121 122 return m_host == other.m_host && m_protocol == other.m_protocol && m_port == other.m_port; 133 // Here are two cases where we should permit access: 134 // 135 // 1) Neither document has set document.domain. In this case, we insist 136 // that the scheme, host, and port of the URLs match. 137 // 138 // 2) Both documents have set document.domain. In this case, we insist 139 // that the documents have set document.domain to the same value and 140 // that the scheme of the URLs match. 141 // 142 // This matches the behavior of Firefox 2 and Internet Explorer 6. 143 // 144 // Internet Explorer 7 and Opera 9 are more strict in that they require 145 // the port numbers to match when both pages have document.domain set. 146 // 147 // FIXME: Evaluate whether we can tighten this policy to require matched 148 // port numbers. 149 // 150 // Opera 9 allows access when only one page has set document.domain, but 151 // this is a security vulnerability. 152 153 if (m_protocol == other->m_protocol) { 154 if (!m_domainWasSetInDOM && !other->m_domainWasSetInDOM) { 155 if (m_host == other->m_host && m_port == other->m_port) 156 return true; 157 } 158 if (m_domainWasSetInDOM && other->m_domainWasSetInDOM) { 159 if (m_host == other->m_host) 160 return true; 161 } 162 } 163 164 return false; 123 165 } 124 166 -
trunk/WebCore/platform/SecurityOrigin.h
r27821 r28912 30 30 #define SecurityOrigin_h 31 31 32 #include <wtf/RefCounted.h> 33 #include <wtf/PassRefPtr.h> 34 32 35 #include "PlatformString.h" 33 36 … … 38 41 class SecurityOriginData; 39 42 40 class SecurityOrigin {43 class SecurityOrigin : public RefCounted<SecurityOrigin> { 41 44 public: 42 SecurityOrigin();45 static PassRefPtr<SecurityOrigin> createForFrame(Frame*); 43 46 44 void setForFrame(Frame*);45 47 void setDomainFromDOM(const String& newDomain); 48 String domain() const { return m_host; } 46 49 47 bool canAccess(const SecurityOrigin &) const;50 bool canAccess(const SecurityOrigin*) const; 48 51 bool isSecureTransitionTo(const KURL&) const; 49 52 … … 53 56 54 57 private: 58 SecurityOrigin(); 59 bool isEmpty() const; 60 55 61 void clear(); 56 bool isEmpty() const;62 void setForURL(const KURL& url); 57 63 58 64 String m_protocol; -
trunk/WebCore/storage/Database.cpp
r28502 r28912 98 98 if (!DatabaseTracker::tracker().canEstablishDatabase(document, name, displayName, estimatedSize)) { 99 99 // There should be an exception raised here in addition to returning a null Database object. The question has been raised with the WHATWG 100 LOG(StorageAPI, "Database %s for origin %s not allowed to be established", name.ascii().data(), document->securityOrigin() .toString().ascii().data());100 LOG(StorageAPI, "Database %s for origin %s not allowed to be established", name.ascii().data(), document->securityOrigin()->toString().ascii().data()); 101 101 return 0; 102 102 } … … 109 109 } 110 110 111 DatabaseTracker::tracker().setDatabaseDetails(document->securityOrigin() .securityOriginData(), name, displayName, estimatedSize);111 DatabaseTracker::tracker().setDatabaseDetails(document->securityOrigin()->securityOriginData(), name, displayName, estimatedSize); 112 112 113 113 if (Page* page = document->frame()->page()) … … 132 132 initializeThreading(); 133 133 134 m_guid = guidForOriginAndName(m_securityOrigin .toString(), name);134 m_guid = guidForOriginAndName(m_securityOrigin->toString(), name); 135 135 136 136 { … … 149 149 ASSERT(m_databaseThread); 150 150 151 m_filename = DatabaseTracker::tracker().fullPathForDatabase(m_securityOrigin .securityOriginData(), m_name);151 m_filename = DatabaseTracker::tracker().fullPathForDatabase(m_securityOrigin->securityOriginData(), m_name); 152 152 } 153 153 … … 551 551 { 552 552 // Return a deep copy for ref counting thread safety 553 return m_securityOrigin .securityOriginData().copy();553 return m_securityOrigin->securityOriginData().copy(); 554 554 } 555 555 -
trunk/WebCore/storage/Database.h
r28502 r28912 45 45 #include <wtf/OwnPtr.h> 46 46 #include <wtf/PassRefPtr.h> 47 #include <wtf/RefPtr.h> 47 48 #include <wtf/Deque.h> 48 49 … … 126 127 127 128 Document* m_document; 128 SecurityOriginm_securityOrigin;129 RefPtr<SecurityOrigin> m_securityOrigin; 129 130 String m_name; 130 131 int m_guid; … … 141 142 142 143 #ifndef NDEBUG 143 String databaseDebugName() const { return m_securityOrigin .toString() + "::" + m_name; }144 String databaseDebugName() const { return m_securityOrigin->toString() + "::" + m_name; } 144 145 #endif 145 146 -
trunk/WebCore/storage/DatabaseTracker.cpp
r28829 r28912 126 126 bool DatabaseTracker::canEstablishDatabase(Document* document, const String& name, const String& displayName, unsigned long estimatedSize) 127 127 { 128 SecurityOriginData originData = document->securityOrigin() .securityOriginData();128 SecurityOriginData originData = document->securityOrigin()->securityOriginData(); 129 129 130 130 // If this origin has no databases yet, establish an entry in the tracker database with the default quota -
trunk/WebCore/storage/SQLTransaction.cpp
r28543 r28912 335 335 // The commit was successful, notify the delegates if the transaction modified this database 336 336 if (m_modifiedDatabase) 337 DatabaseTracker::tracker().scheduleNotifyDatabaseChanged(m_database->m_securityOrigin .securityOriginData(), m_database->m_name);337 DatabaseTracker::tracker().scheduleNotifyDatabaseChanged(m_database->m_securityOrigin->securityOriginData(), m_database->m_name); 338 338 339 339 // Transaction Step 10 - End transaction steps … … 392 392 } else if (m_modifiedDatabase) { 393 393 // But if the commit was successful, notify the delegates if the transaction modified this database 394 DatabaseTracker::tracker().scheduleNotifyDatabaseChanged(m_database->m_securityOrigin .securityOriginData(), m_database->m_name);394 DatabaseTracker::tracker().scheduleNotifyDatabaseChanged(m_database->m_securityOrigin->securityOriginData(), m_database->m_name); 395 395 } 396 396
Note: See TracChangeset
for help on using the changeset viewer.