Don't set document.domain to an IP address fragment
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Aug 2016 18:57:13 +0000 (18:57 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Aug 2016 18:57:13 +0000 (18:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=126045
<rdar://problem/27331794>

Reviewed by Daniel Bates.

Source/WebCore:

This patch matches the following Blink one:
https://chromium.googlesource.com/chromium/blink/+/b19a57fdb323d5a80d3a1cb0a6b343558c4237b0

IP address octets should not be treated as subdomains when setting
document.domain. The specs say:
'The domain attribute's setter must run these steps: ...
7. If host is not equal to effectiveDomain, then run these substeps:
    1. If host or effectiveDomain is not a domain, then throw a
    "SecurityError" DOMException.'
https://html.spec.whatwg.org/multipage/browsers.html#relaxing-the-same-origin-restriction
Last Updated 5 August 2016

'A host is a domain, an IPv4 address, or an IPv6 address.'
https://url.spec.whatwg.org/#concept-domain
Last Updated 28 July 2016

Test: http/tests/security/set-domain-remove-subdomain-for-ip-address.html

* dom/Document.cpp:
(WebCore::Document::setDomain):
    Now checks whether the security origin is allowed to remove
    subdomains. If not, it throws a security error.
* page/OriginAccessEntry.cpp:
(WebCore::OriginAccessEntry::OriginAccessEntry):
    Constructor now expects an IP address setting.
(WebCore::OriginAccessEntry::matchesOrigin):
    Now also checks whether the host in an IP address and returns
    false if IP addresses aren't configured to be treated as domains.
* page/OriginAccessEntry.h:
    Introduced new enum for IP address setting.
    Constructor now expects an IP address setting.
(WebCore::OriginAccessEntry::ipAddressSettings):
    New getter.
(WebCore::operator==):
    Now also requires IP address settings to match.
* page/SecurityPolicy.cpp:
(WebCore::SecurityPolicy::addOriginAccessWhitelistEntry):
    Changes to match OriginAccessEntry's new constructor.
(WebCore::SecurityPolicy::removeOriginAccessWhitelistEntry):
    Changes to match OriginAccessEntry's new constructor.
* page/Settings.in:
    Added a setting to allow IP address octets to be treated as
    subdomains. This way our existing tests setting document.domain
    still work.

LayoutTests:

IP address octets should not be treated as subdomains when
setting document.domain.

* http/tests/security/aboutBlank/security-context-alias.html:
    Now enables the new setting treatIPAddressesAsDomains.
* http/tests/security/aboutBlank/security-context-grandchildren-alias.html:
    Now enables the new setting treatIPAddressesAsDomains.
* http/tests/security/postMessage/origin-unaffected-by-document-domain.html:
    Now enables the new setting treatIPAddressesAsDomains.
* http/tests/security/set-domain-remove-subdomain-for-ip-address-expected.txt: Added.
* http/tests/security/set-domain-remove-subdomain-for-ip-address.html: Added.
* http/tests/workers/worker-document-domain-security.html:
    Now enables the new setting treatIPAddressesAsDomains.
* http/tests/xmlhttprequest/document-domain-set.html:
    Now enables the new setting treatIPAddressesAsDomains.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204181 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/aboutBlank/security-context-alias.html
LayoutTests/http/tests/security/aboutBlank/security-context-grandchildren-alias.html
LayoutTests/http/tests/security/postMessage/origin-unaffected-by-document-domain.html
LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address.html [new file with mode: 0644]
LayoutTests/http/tests/workers/worker-document-domain-security.html
LayoutTests/http/tests/xmlhttprequest/document-domain-set.html
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/OriginAccessEntry.cpp
Source/WebCore/page/OriginAccessEntry.h
Source/WebCore/page/SecurityPolicy.cpp
Source/WebCore/page/Settings.in

index 5ac4835..3f4b0ca 100644 (file)
@@ -1,3 +1,27 @@
+2016-08-05  John Wilander  <wilander@apple.com>
+
+        Don't set document.domain to an IP address fragment
+        https://bugs.webkit.org/show_bug.cgi?id=126045
+        <rdar://problem/27331794>
+
+        Reviewed by Daniel Bates.
+
+        IP address octets should not be treated as subdomains when
+        setting document.domain.
+
+        * http/tests/security/aboutBlank/security-context-alias.html:
+            Now enables the new setting treatIPAddressesAsDomains.
+        * http/tests/security/aboutBlank/security-context-grandchildren-alias.html:
+            Now enables the new setting treatIPAddressesAsDomains.
+        * http/tests/security/postMessage/origin-unaffected-by-document-domain.html:
+            Now enables the new setting treatIPAddressesAsDomains.
+        * http/tests/security/set-domain-remove-subdomain-for-ip-address-expected.txt: Added.
+        * http/tests/security/set-domain-remove-subdomain-for-ip-address.html: Added.
+        * http/tests/workers/worker-document-domain-security.html:
+            Now enables the new setting treatIPAddressesAsDomains.
+        * http/tests/xmlhttprequest/document-domain-set.html:
+            Now enables the new setting treatIPAddressesAsDomains.
+
 2016-08-05  Chris Dumez  <cdumez@apple.com>
 
         Window's named properties should be exposed on a WindowProperties object in its prototype
index a89acc2..6a79c2e 100644 (file)
@@ -7,6 +7,9 @@
 if (window.testRunner)
   testRunner.dumpAsText();
 
+if (window.internals)
+    window.internals.settings.setTreatIPAddressAsDomain(true);
+
 document.write('--- Test begins ---\n');
 document.write('* "about:blank"\n');
 document.write('document.domain = ' + frames[0].document.domain + '\n');
index bea6ad0..a55621c 100644 (file)
@@ -6,6 +6,9 @@ if (window.testRunner) {
   testRunner.waitUntilDone();
 }
 
+if (window.internals)
+    window.internals.settings.setTreatIPAddressAsDomain(true);
+
 function log(msg) {
   var line = document.createElement('div');
   line.appendChild(document.createTextNode(msg));
index fdbce19..431c032 100644 (file)
@@ -8,6 +8,9 @@ if (window.testRunner) {
     testRunner.waitUntilDone();
 }
 
+if (window.internals)
+    window.internals.settings.setTreatIPAddressAsDomain(true);
+
 addEventListener("message", recv, false);
 
 function test() {
diff --git a/LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address-expected.txt b/LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address-expected.txt
new file mode 100644 (file)
index 0000000..c7b918b
--- /dev/null
@@ -0,0 +1,11 @@
+Test whether a page loaded straight from an IP address can set document.domain and wrongly treat IP address octets as subdomains.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.domain = '0.0.1' threw exception SecurityError (DOM Exception 18): The operation is insecure..
+PASS document.domain is "127.0.0.1"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address.html b/LayoutTests/http/tests/security/set-domain-remove-subdomain-for-ip-address.html
new file mode 100644 (file)
index 0000000..8c56662
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Try to set document.domain and wrongly treat IP address octets as subdomains</title>
+    <script src="../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+    description("Test whether a page loaded straight from an IP address can set document.domain and wrongly treat IP address octets as subdomains.");
+
+    if (document.domain != "127.0.0.1") {
+        document.location.hostname = "127.0.0.1";
+    }
+
+    shouldThrow("document.domain = '0.0.1'", "'SecurityError (DOM Exception 18): The operation is insecure.'");
+    shouldBeEqualToString("document.domain", "127.0.0.1");
+</script>
+<script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index 2730a54..39fca8d 100644 (file)
@@ -5,6 +5,9 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 
+if (window.internals)
+    window.internals.settings.setTreatIPAddressAsDomain(true);
+
 function log(message)
 {
     document.getElementById("result").innerHTML += message + "<br>";
index b0cdfd3..f45d48b 100644 (file)
@@ -5,6 +5,9 @@
 if (window.testRunner)
   testRunner.dumpAsText();
 
+if (window.internals)
+    window.internals.settings.setTreatIPAddressAsDomain(true);
+
 document.domain = '0.0.1';
 
 document.write('Waiting...\n');
index d35fe9e..1fa3fd4 100644 (file)
@@ -1,3 +1,56 @@
+2016-08-05  John Wilander  <wilander@apple.com>
+
+        Don't set document.domain to an IP address fragment
+        https://bugs.webkit.org/show_bug.cgi?id=126045
+        <rdar://problem/27331794>
+
+        Reviewed by Daniel Bates.
+
+        This patch matches the following Blink one:
+        https://chromium.googlesource.com/chromium/blink/+/b19a57fdb323d5a80d3a1cb0a6b343558c4237b0
+
+        IP address octets should not be treated as subdomains when setting
+        document.domain. The specs say:
+        'The domain attribute's setter must run these steps: ...
+        7. If host is not equal to effectiveDomain, then run these substeps:
+            1. If host or effectiveDomain is not a domain, then throw a
+            "SecurityError" DOMException.'
+        https://html.spec.whatwg.org/multipage/browsers.html#relaxing-the-same-origin-restriction
+        Last Updated 5 August 2016
+
+        'A host is a domain, an IPv4 address, or an IPv6 address.'
+        https://url.spec.whatwg.org/#concept-domain
+        Last Updated 28 July 2016
+
+        Test: http/tests/security/set-domain-remove-subdomain-for-ip-address.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::setDomain):
+            Now checks whether the security origin is allowed to remove
+            subdomains. If not, it throws a security error.
+        * page/OriginAccessEntry.cpp:
+        (WebCore::OriginAccessEntry::OriginAccessEntry):
+            Constructor now expects an IP address setting.
+        (WebCore::OriginAccessEntry::matchesOrigin):
+            Now also checks whether the host in an IP address and returns
+            false if IP addresses aren't configured to be treated as domains.
+        * page/OriginAccessEntry.h:
+            Introduced new enum for IP address setting.
+            Constructor now expects an IP address setting.
+        (WebCore::OriginAccessEntry::ipAddressSettings):
+            New getter.
+        (WebCore::operator==):
+            Now also requires IP address settings to match.
+        * page/SecurityPolicy.cpp:
+        (WebCore::SecurityPolicy::addOriginAccessWhitelistEntry):
+            Changes to match OriginAccessEntry's new constructor.
+        (WebCore::SecurityPolicy::removeOriginAccessWhitelistEntry):
+            Changes to match OriginAccessEntry's new constructor.
+        * page/Settings.in:
+            Added a setting to allow IP address octets to be treated as
+            subdomains. This way our existing tests setting document.domain
+            still work.
+
 2016-08-05  Chris Dumez  <cdumez@apple.com>
 
         Window's named properties should be exposed on a WindowProperties object in its prototype
index 5642f25..c56bd96 100644 (file)
 #include "NodeIterator.h"
 #include "NodeRareData.h"
 #include "NodeWithIndex.h"
+#include "OriginAccessEntry.h"
 #include "OverflowEvent.h"
 #include "PageConsoleClient.h"
 #include "PageGroup.h"
@@ -4497,6 +4498,13 @@ void Document::setDomain(const String& newDomain, ExceptionCode& ec)
         return;
     }
 
+    OriginAccessEntry::IPAddressSetting ipAddressSetting = settings() && settings()->treatIPAddressAsDomain() ? OriginAccessEntry::TreatIPAddressAsDomain : OriginAccessEntry::TreatIPAddressAsIPAddress;
+    OriginAccessEntry accessEntry(securityOrigin()->protocol(), newDomain, OriginAccessEntry::AllowSubdomains, ipAddressSetting);
+    if (!accessEntry.matchesOrigin(*securityOrigin())) {
+        ec = SECURITY_ERR;
+        return;
+    }
+
     String test = domain();
     // Check that it's a subdomain, not e.g. "ebkit.org"
     if (test[oldLength - newLength - 1] != '.') {
index 8c123ae..9eaecfd 100644 (file)
 
 namespace WebCore {
     
-OriginAccessEntry::OriginAccessEntry(const String& protocol, const String& host, SubdomainSetting subdomainSetting)
+OriginAccessEntry::OriginAccessEntry(const String& protocol, const String& host, SubdomainSetting subdomainSetting, IPAddressSetting ipAddressSetting)
     : m_protocol(protocol.convertToASCIILowercase())
     , m_host(host.convertToASCIILowercase())
     , m_subdomainSettings(subdomainSetting)
+    , m_ipAddressSettings(ipAddressSetting)
 {
     ASSERT(subdomainSetting == AllowSubdomains || subdomainSetting == DisallowSubdomains);
 
@@ -65,9 +66,9 @@ bool OriginAccessEntry::matchesOrigin(const SecurityOrigin& origin) const
     // Otherwise we can only match if we're matching subdomains.
     if (m_subdomainSettings == DisallowSubdomains)
         return false;
-    
+
     // Don't try to do subdomain matching on IP addresses.
-    if (m_hostIsIPAddress)
+    if (m_hostIsIPAddress && m_ipAddressSettings == TreatIPAddressAsIPAddress)
         return false;
     
     // Match subdomains.
index 06614e1..96c2b9d 100644 (file)
@@ -44,24 +44,34 @@ public:
         DisallowSubdomains
     };
 
+    enum IPAddressSetting {
+        TreatIPAddressAsDomain,
+        TreatIPAddressAsIPAddress
+    };
+
     // If host is empty string and SubdomainSetting is AllowSubdomains, the entry will match all domains in the specified protocol.
-    OriginAccessEntry(const String& protocol, const String& host, SubdomainSetting);
+    OriginAccessEntry(const String& protocol, const String& host, SubdomainSetting, IPAddressSetting);
     bool matchesOrigin(const SecurityOrigin&) const;
 
     const String& protocol() const { return m_protocol; }
     const String& host() const { return m_host; }
     SubdomainSetting subdomainSettings() const { return m_subdomainSettings; }
+    IPAddressSetting ipAddressSettings() const { return m_ipAddressSettings; }
 
 private:
     String m_protocol;
     String m_host;
     SubdomainSetting m_subdomainSettings;
+    IPAddressSetting m_ipAddressSettings;
     bool m_hostIsIPAddress;
 };
 
 inline bool operator==(const OriginAccessEntry& a, const OriginAccessEntry& b)
 {
-    return equalIgnoringASCIICase(a.protocol(), b.protocol()) && equalIgnoringASCIICase(a.host(), b.host()) && a.subdomainSettings() == b.subdomainSettings();
+    return equalIgnoringASCIICase(a.protocol(), b.protocol())
+        && equalIgnoringASCIICase(a.host(), b.host())
+        && a.subdomainSettings() == b.subdomainSettings()
+        && a.ipAddressSettings() == b.ipAddressSettings();
 }
 
 inline bool operator!=(const OriginAccessEntry& a, const OriginAccessEntry& b)
index 5db273d..7986342 100644 (file)
@@ -139,7 +139,7 @@ void SecurityPolicy::addOriginAccessWhitelistEntry(const SecurityOrigin& sourceO
         result.iterator->value = std::make_unique<OriginAccessWhiteList>();
 
     OriginAccessWhiteList* list = result.iterator->value.get();
-    list->append(OriginAccessEntry(destinationProtocol, destinationDomain, allowDestinationSubdomains ? OriginAccessEntry::AllowSubdomains : OriginAccessEntry::DisallowSubdomains));
+    list->append(OriginAccessEntry(destinationProtocol, destinationDomain, allowDestinationSubdomains ? OriginAccessEntry::AllowSubdomains : OriginAccessEntry::DisallowSubdomains, OriginAccessEntry::TreatIPAddressAsIPAddress));
 }
 
 void SecurityPolicy::removeOriginAccessWhitelistEntry(const SecurityOrigin& sourceOrigin, const String& destinationProtocol, const String& destinationDomain, bool allowDestinationSubdomains)
@@ -156,7 +156,7 @@ void SecurityPolicy::removeOriginAccessWhitelistEntry(const SecurityOrigin& sour
         return;
 
     OriginAccessWhiteList& list = *it->value;
-    OriginAccessEntry originAccessEntry(destinationProtocol, destinationDomain, allowDestinationSubdomains ? OriginAccessEntry::AllowSubdomains : OriginAccessEntry::DisallowSubdomains);
+    OriginAccessEntry originAccessEntry(destinationProtocol, destinationDomain, allowDestinationSubdomains ? OriginAccessEntry::AllowSubdomains : OriginAccessEntry::DisallowSubdomains, OriginAccessEntry::TreatIPAddressAsIPAddress);
     if (!list.removeFirst(originAccessEntry))
         return;
 
index 98dcf86..cbd5ad8 100644 (file)
@@ -271,3 +271,5 @@ selectionPaintingWithoutSelectionGapsEnabled initial=false
 shouldConvertInvalidURLsToBlank initial=true
 
 springTimingFunctionEnabled initial=false
+
+treatIPAddressAsDomain initial=false