Block connections to 0.0.0.0

https://bugs.webkit.org/show_bug.cgi?id=275224 rdar://125913679 Reviewed by Alex Christensen. This patch blocks connections to 0.0.0.0 and [::], as per RFC 6890 [0]. That spec says that these addresses may only be used as source addresses, not destinations. Requesting connections to either of these addresses is generally confusing, and likely not motivated by a good reason. Coincidentally, Blink seems to be making a similar change [1], and their use counters are showing below 0.001%. We can't exactly extrapolate from that, but it's a good indicator that this is safe. [0] https://www.rfc-editor.org/rfc/rfc6890#section-2.2.3 [1] https://groups.google.com/a/chromium.org/g/blink-dev/c/9uymCQNGVgw * LayoutTests/http/tests/media/video-error-all-zero-address-blocked-expected.txt: Added. * LayoutTests/http/tests/media/video-error-all-zero-address-blocked.html: Added. * LayoutTests/http/tests/security/block-iframe-to-all-zero-address-expected.txt: Added. * LayoutTests/http/tests/security/block-iframe-to-all-zero-address.html: Added. * LayoutTests/http/tests/security/block-popup-to-all-zero-address-expected.txt: Added. * LayoutTests/http/tests/security/block-popup-to-all-zero-address.html: Added. * LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address-expected.txt: Added. * LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address.html: Added. * LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked-expected.txt: Added. * LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked.html: Added. * LayoutTests/platform/wincairo/TestExpectations: * Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp: (WebCore::RTCPeerConnection::iceServersFromConfiguration): * Source/WebCore/Modules/websockets/WebSocket.cpp: (WebCore::WebSocket::connect): * Source/WebCore/html/HTMLMediaElement.cpp: (WebCore::HTMLMediaElement::isSafeToLoadURL const): * Source/WebCore/loader/DocumentLoader.cpp: (WebCore::DocumentLoader::willSendRequest): * Source/WebCore/loader/FrameLoader.cpp: (WebCore::FrameLoader::loadFrameRequest): (WebCore::FrameLoader::reportBlockedLoadFailed): * Source/WebCore/loader/ResourceLoader.cpp: (WebCore::ResourceLoader::init): * Source/WebCore/loader/SubframeLoader.cpp: (WebCore::FrameLoader::SubframeLoader::pluginIsLoadable): (WebCore::FrameLoader::SubframeLoader::loadSubframe): * Source/WebCore/loader/cache/CachedResourceLoader.cpp: (WebCore::CachedResourceLoader::requestResource): * Source/WebCore/platform/network/DNS.cpp: (WebCore::isIPAddressDisallowed): * Source/WebCore/platform/network/DNS.h: * Source/WebCore/platform/network/ResourceHandle.cpp: (WebCore::ResourceHandle::ResourceHandle): * Source/WebKit/NetworkProcess/NetworkDataTask.cpp: (WebKit::NetworkDataTask::NetworkDataTask): Canonical link: https://commits.webkit.org/279835@main
WebKit · Jun 7, 2024 · e59cd4a · e59cd4a
1 parent f6a08ca
commit e59cd4a
Show file tree

Hide file tree

Showing 23 changed files with 175 additions and 11 deletions.
diff --git a/LayoutTests/http/tests/media/video-error-all-zero-address-blocked-expected.txt b/LayoutTests/http/tests/media/video-error-all-zero-address-blocked-expected.txt
@@ -0,0 +1,6 @@
+CONSOLE MESSAGE: Not allowed to use restricted network host "0.0.0.0": http://0.0.0.0:8000/media/resources/test.mp4
+PASS Received error
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Test passes if loading video is not successful
diff --git a/LayoutTests/http/tests/media/video-error-all-zero-address-blocked.html b/LayoutTests/http/tests/media/video-error-all-zero-address-blocked.html
@@ -0,0 +1,27 @@
+<head>
+    <script src="/js-test-resources/js-test.js"></script>
+</head>
+<body>
+<p>Test passes if loading video is not successful</p>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    const video = document.createElement("video");
+    video.src = "http://0.0.0.0:8000/media/resources/test.mp4";
+    video.onerror = (e) => {
+        testPassed("Received error");
+        testRunner.notifyDone();
+    }
+    video.onopen = (e) => {
+        testFailed("Received open");
+        testRunner.notifyDone();
+    }
+    video.onmessage = (e) => {
+        testFailed("Received message");
+        testRunner.notifyDone();
+    }
+</script>
+</body>
diff --git a/LayoutTests/http/tests/security/block-iframe-to-all-zero-address-expected.txt b/LayoutTests/http/tests/security/block-iframe-to-all-zero-address-expected.txt
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: Not allowed to use restricted network host "0.0.0.0": http://0.0.0.0:8000/security/resources/target.html
+
+This attempts to load an iframe from the all-zeroes address, which should be blocked.
diff --git a/LayoutTests/http/tests/security/block-iframe-to-all-zero-address.html b/LayoutTests/http/tests/security/block-iframe-to-all-zero-address.html
@@ -0,0 +1,11 @@
+<html>
+<body>
+<iframe src="http://0.0.0.0:8000/security/resources/target.html"></iframe></br>
+<script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+    }
+</script>
+<p>This attempts to load an iframe from the all-zeroes address, which should be blocked.</p>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/block-popup-to-all-zero-address-expected.txt b/LayoutTests/http/tests/security/block-popup-to-all-zero-address-expected.txt
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: Not allowed to use restricted network host "0.0.0.0": http://0.0.0.0:8000/security/resources/target.html
+This attempts to load an iframe from the all-zeroes address, which should be blocked.
diff --git a/LayoutTests/http/tests/security/block-popup-to-all-zero-address.html b/LayoutTests/http/tests/security/block-popup-to-all-zero-address.html
@@ -0,0 +1,12 @@
+<html>
+<body>
+<script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+    }
+    window.open("http://0.0.0.0:8000/security/resources/target.html");
+
+</script>
+<p>This attempts to load an iframe from the all-zeroes address, which should be blocked.</p>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address-expected.txt b/LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address-expected.txt
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: Not allowed to use restricted network host "0.0.0.0": http://0.0.0.0:8000/security/resources/file-redirect-target.html
+
+This attempts to open a redirect link to the all-zeroes address, which should be blocked.
diff --git a/LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address.html b/LayoutTests/http/tests/security/redirect-BLOCKED-to-all-zero-address.html
@@ -0,0 +1,11 @@
+<html>
+<body>
+<iframe src="http://127.0.0.1:8000/resources/redirect.py?url=http://0.0.0.0:8000/security/resources/file-redirect-target.html"></iframe></br>
+<script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+    }
+</script>
+<p>This attempts to open a redirect link to the all-zeroes address, which should be blocked.</p>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked-expected.txt b/LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked-expected.txt
@@ -0,0 +1,6 @@
+CONSOLE MESSAGE: WebSocket address 0.0.0.0 blocked
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS Received error
+Test passes if websocket connection is not successful
diff --git a/LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked.html b/LayoutTests/http/tests/websocket/connection-to-all-zero-address-blocked.html
@@ -0,0 +1,26 @@
+<head>
+    <script src="/js-test-resources/js-test.js"></script>
+</head>
+<body>
+<p>Test passes if websocket connection is not successful</p>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    const socket = new WebSocket("ws://0.0.0.0:8880");
+    socket.onerror = (e) => {
+        testPassed("Received error");
+        testRunner.notifyDone();
+    }
+    socket.onopen = (e) => {
+        testFailed("Received open");
+        testRunner.notifyDone();
+    }
+    socket.onmessage = (e) => {
+        testFailed("Received message");
+        testRunner.notifyDone();
+    }
+</script>
+</body>
diff --git a/LayoutTests/platform/wincairo/TestExpectations b/LayoutTests/platform/wincairo/TestExpectations
@@ -910,6 +910,12 @@ http/tests/websocket/tests/hybi/multiple-connections-limit.html [ Skip ] # Timeo
 # Timeout on local PC
 http/tests/websocket/tests/hybi/frame-lengths.html [ Pass Timeout ]
 
+# Connections need additional validation
+http/tests/security/block-iframe-to-all-zero-address.html [ Failure ]
+http/tests/security/block-popup-to-all-zero-address.html [ Failure ]
+http/tests/security/redirect-BLOCKED-to-all-zero-address.html [ Failure ]
+http/tests/websocket/connection-to-all-zero-address-blocked.html [ Failure ]
+
 # Failure on Buildbot
 http/tests/websocket/tests/hybi/inspector/binary.html [ Pass Failure ]
 

diff --git a/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp b/Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp
@@ -36,6 +36,7 @@
 
 #if ENABLE(WEB_RTC)
 
+#include "DNS.h"
 #include "Document.h"
 #include "Event.h"
 #include "EventNames.h"
@@ -408,7 +409,7 @@ ExceptionOr<Vector<MediaEndpointConfiguration::IceServerInfo>> RTCPeerConnection
 
             urls.removeAllMatching([&](auto& urlString) {
                 URL url { URL { }, urlString };
-                if (url.path().endsWithIgnoringASCIICase(".local"_s) || !portAllowed(url)) {
+                if (url.path().endsWithIgnoringASCIICase(".local"_s) || !portAllowed(url) || isIPAddressDisallowed(url)) {
                     queueTaskToDispatchEvent(*this, TaskSource::MediaElement, RTCPeerConnectionIceErrorEvent::create(Event::CanBubble::No, Event::IsCancelable::No, { }, { }, WTFMove(urlString), 701, "URL is not allowed"_s));
                     return true;
                 }

diff --git a/Source/WebCore/Modules/websockets/WebSocket.cpp b/Source/WebCore/Modules/websockets/WebSocket.cpp
@@ -35,6 +35,7 @@
 #include "Blob.h"
 #include "CloseEvent.h"
 #include "ContentSecurityPolicy.h"
+#include "DNS.h"
 #include "Document.h"
 #include "Event.h"
 #include "EventListener.h"
@@ -257,9 +258,11 @@ ExceptionOr<void> WebSocket::connect(const String& url, const Vector<String>& pr
 
     contentSecurityPolicy->upgradeInsecureRequestIfNeeded(m_url, ContentSecurityPolicy::InsecureRequestType::Load);
 
-    if (!portAllowed(m_url)) {
+    if (!portAllowed(m_url) || isIPAddressDisallowed(m_url)) {
         String message;
-        if (m_url.port())
+        if (isIPAddressDisallowed(m_url))
+            message = makeString("WebSocket address "_s, m_url.host(), " blocked"_s);
+        else if (m_url.port())
             message = makeString("WebSocket port "_s, m_url.port().value(), " blocked"_s);
         else
             message = "WebSocket without port blocked"_s;

diff --git a/Source/WebCore/html/HTMLMediaElement.cpp b/Source/WebCore/html/HTMLMediaElement.cpp
@@ -48,6 +48,7 @@
 #include "ContentSecurityPolicy.h"
 #include "ContentType.h"
 #include "CookieJar.h"
+#include "DNS.h"
 #include "DeprecatedGlobalSettings.h"
 #include "DiagnosticLoggingClient.h"
 #include "DiagnosticLoggingKeys.h"
@@ -2572,12 +2573,16 @@ bool HTMLMediaElement::isSafeToLoadURL(const URL& url, InvalidURLAction actionIf
         return false;
     }
 
-    if (!portAllowed(url)) {
+    if (!portAllowed(url) || isIPAddressDisallowed(url)) {
         if (actionIfInvalid == Complain) {
             if (frame)
                 FrameLoader::reportBlockedLoadFailed(*frame, url);
-            if (shouldLog)
-                ERROR_LOG(LOGIDENTIFIER, url , " was rejected because the port is not allowed");
+            if (shouldLog) {
+                if (isIPAddressDisallowed(url))
+                    ERROR_LOG(LOGIDENTIFIER, url , " was rejected because the address not allowed");
+                else
+                    ERROR_LOG(LOGIDENTIFIER, url , " was rejected because the port is not allowed");
+            }
         }
         return false;
     }

diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
@@ -41,6 +41,7 @@
 #include "ContentSecurityPolicy.h"
 #include "CrossOriginOpenerPolicy.h"
 #include "CustomHeaderFields.h"
+#include "DNS.h"
 #include "DocumentInlines.h"
 #include "DocumentParser.h"
 #include "DocumentWriter.h"
@@ -701,6 +702,13 @@ void DocumentLoader::willSendRequest(ResourceRequest&& newRequest, const Resourc
             cancelMainResourceLoad(frameLoader()->blockedError(newRequest));
             return completionHandler(WTFMove(newRequest));
         }
+        if (isIPAddressDisallowed(newRequest.url())) {
+            DOCUMENTLOADER_RELEASE_LOG("willSendRequest: canceling - redirecting to a URL with a disallowed IP address");
+            if (m_frame)
+                FrameLoader::reportBlockedLoadFailed(*m_frame, newRequest.url());
+            cancelMainResourceLoad(frameLoader()->blockedError(newRequest));
+            return completionHandler(WTFMove(newRequest));
+        }
     }
 
     ASSERT(m_frame);

diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
@@ -51,6 +51,7 @@
 #include "ContentSecurityPolicy.h"
 #include "CrossOriginAccessControl.h"
 #include "CrossOriginEmbedderPolicy.h"
+#include "DNS.h"
 #include "DatabaseManager.h"
 #include "DiagnosticLoggingClient.h"
 #include "DiagnosticLoggingKeys.h"
@@ -1384,6 +1385,12 @@ void FrameLoader::loadFrameRequest(FrameLoadRequest&& request, Event* event, Ref
         reportBlockedLoadFailed(frame, url);
         return;
     }
+
+    if (isIPAddressDisallowed(url)) {
+        FRAMELOADER_RELEASE_LOG(ResourceLoading, "loadFrameRequest: canceling - IP address is not allowed");
+        reportBlockedLoadFailed(frame, url);
+        return;
+    }
 
     URL argsReferrer;
     String argsReferrerString = request.resourceRequest().httpReferrer();
@@ -1841,7 +1848,8 @@ void FrameLoader::reportLocalLoadFailed(LocalFrame* frame, const String& url)
 void FrameLoader::reportBlockedLoadFailed(LocalFrame& frame, const URL& url)
 {
     ASSERT(!url.isEmpty());
-    auto message = makeString("Not allowed to use restricted network port "_s, url.port().value(), ": "_s, url.stringCenterEllipsizedToLength());
+    auto restrictedHostPort = isIPAddressDisallowed(url) ? makeString("host \""_s, url.host(), "\""_s) : makeString("port "_s, url.port().value());
+    auto message = makeString("Not allowed to use restricted network "_s, restrictedHostPort, ": "_s, url.stringCenterEllipsizedToLength());
     frame.protectedDocument()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message);
 }
 

diff --git a/Source/WebCore/loader/ResourceLoader.cpp b/Source/WebCore/loader/ResourceLoader.cpp
@@ -33,6 +33,7 @@
 #include "ApplicationCacheHost.h"
 #include "AuthenticationChallenge.h"
 #include "ContentRuleListResults.h"
+#include "DNS.h"
 #include "DataURLDecoder.h"
 #include "DiagnosticLoggingClient.h"
 #include "DiagnosticLoggingKeys.h"
@@ -166,6 +167,13 @@ void ResourceLoader::init(ResourceRequest&& clientRequest, CompletionHandler<voi
         return completionHandler(false);
     }
 
+    if (isIPAddressDisallowed(clientRequest.url())) {
+        RESOURCELOADER_RELEASE_LOG("init: Cancelling load to disallowed IP address.");
+        FrameLoader::reportBlockedLoadFailed(*protectedFrame(), clientRequest.url());
+        releaseResources();
+        return completionHandler(false);
+    }
+
     // The various plug-in implementations call directly to ResourceLoader::load() instead of piping requests
     // through FrameLoader. As a result, they miss the FrameLoader::updateRequestAndAddExtraFields() step which sets
     // up the 1st party for cookies URL and Same-Site info. Until plug-in implementations can be reigned in

diff --git a/Source/WebCore/loader/SubframeLoader.cpp b/Source/WebCore/loader/SubframeLoader.cpp
@@ -34,6 +34,7 @@
 #include "SubframeLoader.h"
 
 #include "ContentSecurityPolicy.h"
+#include "DNS.h"
 #include "DiagnosticLoggingClient.h"
 #include "DiagnosticLoggingKeys.h"
 #include "DocumentInlines.h"
@@ -138,7 +139,7 @@ bool FrameLoader::SubframeLoader::pluginIsLoadable(const URL& url)
             return false;
         }
 
-        if (!portAllowed(url)) {
+        if (!portAllowed(url) || isIPAddressDisallowed(url)) {
             FrameLoader::reportBlockedLoadFailed(protectedFrame(), url);
             return false;
         }
@@ -287,7 +288,7 @@ RefPtr<LocalFrame> FrameLoader::SubframeLoader::loadSubframe(HTMLFrameOwnerEleme
         return nullptr;
     }
 
-    if (!portAllowed(url)) {
+    if (!portAllowed(url) || isIPAddressDisallowed(url)) {
         FrameLoader::reportBlockedLoadFailed(frame, url);
         return nullptr;
     }

diff --git a/Source/WebCore/loader/cache/CachedResourceLoader.cpp b/Source/WebCore/loader/cache/CachedResourceLoader.cpp
@@ -47,6 +47,7 @@
 #include "CookieJar.h"
 #include "CrossOriginAccessControl.h"
 #include "CustomHeaderFields.h"
+#include "DNS.h"
 #include "DateComponents.h"
 #include "DocumentInlines.h"
 #include "DocumentLoader.h"
@@ -1059,6 +1060,12 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ
         return makeUnexpected(frame->checkedLoader()->blockedError(request.resourceRequest()));
     }
 
+    if (isIPAddressDisallowed(url)) {
+        FrameLoader::reportBlockedLoadFailed(frame, url);
+        CACHEDRESOURCELOADER_RELEASE_LOG_WITH_FRAME("CachedResourceLoader::requestResource URL has a disallowd address", frame.get());
+        return makeUnexpected(frame->checkedLoader()->blockedError(request.resourceRequest()));
+    }
+
     request.updateReferrerPolicy(document() ? document()->referrerPolicy() : ReferrerPolicy::Default);
 
     if (InspectorInstrumentation::willIntercept(frame.ptr(), request.resourceRequest()))

diff --git a/Source/WebCore/platform/network/DNS.cpp b/Source/WebCore/platform/network/DNS.cpp
@@ -64,6 +64,14 @@ void stopResolveDNS(uint64_t identifier)
     WebCore::DNSResolveQueue::singleton().stopResolve(identifier);
 }
 
+// FIXME: Temporary fix until we have rdar://63797758
+bool isIPAddressDisallowed(const URL& url)
+{
+    if (auto address = IPAddress::fromString(url.host().toStringWithoutCopying()))
+        return address->containsOnlyZeros();
+    return false;
+}
+
 bool IPAddress::containsOnlyZeros() const
 {
     return std::visit(WTF::makeVisitor([] (const WTF::HashTableEmptyValueType&) {

diff --git a/Source/WebCore/platform/network/DNS.h b/Source/WebCore/platform/network/DNS.h
@@ -110,6 +110,7 @@ using DNSCompletionHandler = CompletionHandler<void(DNSAddressesOrError&&)>;
 WEBCORE_EXPORT void prefetchDNS(const String& hostname);
 WEBCORE_EXPORT void resolveDNS(const String& hostname, uint64_t identifier, DNSCompletionHandler&&);
 WEBCORE_EXPORT void stopResolveDNS(uint64_t identifier);
+WEBCORE_EXPORT bool isIPAddressDisallowed(const URL&);
 
 } // namespace WebCore
 

diff --git a/Source/WebCore/platform/network/ResourceHandle.cpp b/Source/WebCore/platform/network/ResourceHandle.cpp
@@ -27,6 +27,7 @@
 #include "ResourceHandle.h"
 #include "ResourceHandleInternal.h"
 
+#include "DNS.h"
 #include "Logging.h"
 #include "NetworkingContext.h"
 #include "NotImplemented.h"
@@ -85,7 +86,7 @@ ResourceHandle::ResourceHandle(NetworkingContext* context, const ResourceRequest
         return;
     }
 
-    if (!portAllowed(request.url())) {
+    if (!portAllowed(request.url()) || isIPAddressDisallowed(request.url())) {
         scheduleFailure(BlockedFailure);
         return;
     }

diff --git a/Source/WebKit/NetworkProcess/NetworkDataTask.cpp b/Source/WebKit/NetworkProcess/NetworkDataTask.cpp
@@ -93,7 +93,7 @@ NetworkDataTask::NetworkDataTask(NetworkSession& session, NetworkDataTaskClient&
         return;
     }
 
-    if (!portAllowed(requestWithCredentials.url())) {
+    if (!portAllowed(requestWithCredentials.url()) || isIPAddressDisallowed(requestWithCredentials.url())) {
         scheduleFailure(FailureType::Blocked);
         return;
     }