Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (258630 => 258631)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2020-03-18 14:49:37 UTC (rev 258631)
@@ -1,3 +1,17 @@
+2020-03-18 youenn fablet <[email protected]>
+
+ Make sure a preflight fails if response headers are invalid
+ https://bugs.webkit.org/show_bug.cgi?id=208924
+
+ Reviewed by Alex Christensen.
+
+ * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt: Added.
+ * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html: Added.
+ * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js: Added.
+ (corsPreflightResponseValidation):
+ * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt: Added.
+ * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html: Added.
+
2020-03-17 Frederic Wang <[email protected]>
Update wpt tests imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/the-page
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt (0 => 258631)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt 2020-03-18 14:49:37 UTC (rev 258631)
@@ -0,0 +1,4 @@
+
+PASS Preflight response with a bad Access-Control-Allow-Headers
+PASS Preflight response with a bad Access-Control-Allow-Methods
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html (0 => 258631)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html 2020-03-18 14:49:37 UTC (rev 258631)
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js (0 => 258631)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js 2020-03-18 14:49:37 UTC (rev 258631)
@@ -0,0 +1,34 @@
+// META: script=/common/utils.js
+// META: script=../resources/utils.js
+// META: script=/common/get-host-info.sub.js
+
+function corsPreflightResponseValidation(desc, corsUrl, allowHeaders, allowMethods) {
+ var uuid_token = token();
+ var url = ""
+ var requestInit = {"mode": "cors"};
+ /* Force preflight */
+ requestInit["headers"] = {"x-force-preflight": ""};
+
+ var urlParameters = "?token=" + uuid_token + "&max_age=0";
+ urlParameters += "&allow_headers=x-force-preflight";
+ if (allowHeaders)
+ urlParameters += "," + allowHeaders;
+ if (allowMethods)
+ urlParameters += "&allow_methods="+ allowMethods;
+
+ promise_test(function(test) {
+ return fetch(RESOURCES_DIR + "clean-stash.py?token=" + uuid_token).then(async function(resp) {
+ assert_equals(resp.status, 200, "Clean stash response's status is 200");
+ await promise_rejects_js(test, TypeError, fetch(url + urlParameters, requestInit));
+
+ return fetch(url + urlParameters).then(function(resp) {
+ assert_equals(resp.headers.get("x-did-preflight"), "1", "Preflight request has been made");
+ });
+ });
+ }, desc);
+}
+
+var corsUrl = get_host_info().HTTP_REMOTE_ORIGIN + dirname(location.pathname) + RESOURCES_DIR + "preflight.py";
+corsPreflightResponseValidation("Preflight response with a bad Access-Control-Allow-Headers", corsUrl, "Bad value", null);
+corsPreflightResponseValidation("Preflight response with a bad Access-Control-Allow-Methods", corsUrl, null, "Bad value");
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt (0 => 258631)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt 2020-03-18 14:49:37 UTC (rev 258631)
@@ -0,0 +1,4 @@
+
+PASS Preflight response with a bad Access-Control-Allow-Headers
+PASS Preflight response with a bad Access-Control-Allow-Methods
+
Added: trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html (0 => 258631)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html (rev 0)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html 2020-03-18 14:49:37 UTC (rev 258631)
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
Modified: trunk/Source/WebCore/ChangeLog (258630 => 258631)
--- trunk/Source/WebCore/ChangeLog 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/ChangeLog 2020-03-18 14:49:37 UTC (rev 258631)
@@ -1,3 +1,32 @@
+2020-03-18 youenn fablet <[email protected]>
+
+ Make sure a preflight fails if response headers are invalid
+ https://bugs.webkit.org/show_bug.cgi?id=208924
+
+ Reviewed by Alex Christensen.
+
+ Implement https://fetch.spec.whatwg.org/#cors-preflight-fetch-0 step 7.3.
+ In case header parsing is wrong, fail the preflight with a meaningful message.
+ Update parsing of headers to return an Optional so that parsing error is handled as a nullopt.
+ Minor refactoring to return Expected/Optional for error handlng instead of passing an out parameter.
+ Also, adding preflight cache entry if it is valid, no matter whether preflight succeeds or not.
+
+ Tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html
+ imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html
+
+ * loader/CrossOriginAccessControl.cpp:
+ (WebCore::validatePreflightResponse):
+ * loader/CrossOriginPreflightResultCache.cpp:
+ (WebCore::CrossOriginPreflightResultCacheItem::create):
+ (WebCore::CrossOriginPreflightResultCacheItem::validateMethodAndHeaders const):
+ * loader/CrossOriginPreflightResultCache.h:
+ (WebCore::CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem):
+ * platform/network/HTTPParsers.h:
+ (WebCore::parseAccessControlAllowList):
+ * platform/network/ResourceResponseBase.cpp:
+ (WebCore::ResourceResponseBase::filter):
+ (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
+
2020-03-18 Joonghun Park <[email protected]>
Unreviewed. Remove the build warning below since r258458
Modified: trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp (258630 => 258631)
--- trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/loader/CrossOriginAccessControl.cpp 2020-03-18 14:49:37 UTC (rev 258631)
@@ -264,16 +264,17 @@
if (!accessControlCheckResult)
return accessControlCheckResult;
- auto result = makeUnique<CrossOriginPreflightResultCacheItem>(storedCredentialsPolicy);
- String errorDescription;
- // FIXME: allowsCrossOriginMethod and allowsCrossOriginHeaders should return Expected<void, String> to avoid having an out parameter.
- if (!result->parse(response)
- || !result->allowsCrossOriginMethod(request.httpMethod(), storedCredentialsPolicy, errorDescription)
- || !result->allowsCrossOriginHeaders(request.httpHeaderFields(), storedCredentialsPolicy, errorDescription)) {
- return makeUnexpected(errorDescription);
- }
+ auto result = CrossOriginPreflightResultCacheItem::create(storedCredentialsPolicy, response);
+ if (!result.has_value())
+ return makeUnexpected(WTFMove(result.error()));
- CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), WTFMove(result));
+ auto entry = WTFMove(result.value());
+ auto errorDescription = entry->validateMethodAndHeaders(request.httpMethod(), request.httpHeaderFields());
+ CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), entry.moveToUniquePtr());
+
+ if (errorDescription)
+ return makeUnexpected(WTFMove(*errorDescription));
+
return { };
}
Modified: trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp (258630 => 258631)
--- trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.cpp 2020-03-18 14:49:37 UTC (rev 258631)
@@ -52,11 +52,16 @@
return ok;
}
-bool CrossOriginPreflightResultCacheItem::parse(const ResourceResponse& response)
+Expected<UniqueRef<CrossOriginPreflightResultCacheItem>, String> CrossOriginPreflightResultCacheItem::create(StoredCredentialsPolicy policy, const ResourceResponse& response)
{
- m_methods = parseAccessControlAllowList(response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods));
- m_headers = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders));
+ auto methods = parseAccessControlAllowList(response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods));
+ if (!methods)
+ return makeUnexpected(makeString("Header Access-Control-Allow-Methods has an invalid value: ", response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods)));
+ auto headers = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders));
+ if (!headers)
+ return makeUnexpected(makeString("Header Access-Control-Allow-Headers has an invalid value: ", response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders)));
+
Seconds expiryDelta = 0_s;
if (parseAccessControlMaxAge(response.httpHeaderField(HTTPHeaderName::AccessControlMaxAge), expiryDelta)) {
if (expiryDelta > maxPreflightCacheTimeout)
@@ -64,10 +69,19 @@
} else
expiryDelta = defaultPreflightCacheTimeout;
- m_absoluteExpiryTime = MonotonicTime::now() + expiryDelta;
- return true;
+ return makeUniqueRef<CrossOriginPreflightResultCacheItem>(MonotonicTime::now() + expiryDelta, policy, WTFMove(*methods), WTFMove(*headers));
}
+Optional<String> CrossOriginPreflightResultCacheItem::validateMethodAndHeaders(const String& method, const HTTPHeaderMap& requestHeaders) const
+{
+ String errorDescription;
+ if (!allowsCrossOriginMethod(method, m_storedCredentialsPolicy, errorDescription))
+ return WTFMove(errorDescription);
+ if (!allowsCrossOriginHeaders(requestHeaders, m_storedCredentialsPolicy, errorDescription))
+ return WTFMove(errorDescription);
+ return { };
+}
+
bool CrossOriginPreflightResultCacheItem::allowsCrossOriginMethod(const String& method, StoredCredentialsPolicy storedCredentialsPolicy, String& errorDescription) const
{
if (m_methods.contains(method) || (m_methods.contains("*") && storedCredentialsPolicy != StoredCredentialsPolicy::Use) || isOnAccessControlSimpleRequestMethodWhitelist(method))
Modified: trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h (258630 => 258631)
--- trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h 2020-03-18 14:49:37 UTC (rev 258631)
@@ -27,10 +27,12 @@
#pragma once
#include "StoredCredentialsPolicy.h"
+#include <wtf/Expected.h>
#include <wtf/HashMap.h>
#include <wtf/HashSet.h>
#include <wtf/MonotonicTime.h>
#include <wtf/URLHash.h>
+#include <wtf/UniqueRef.h>
namespace WebCore {
@@ -40,17 +42,17 @@
class CrossOriginPreflightResultCacheItem {
WTF_MAKE_NONCOPYABLE(CrossOriginPreflightResultCacheItem); WTF_MAKE_FAST_ALLOCATED;
public:
- explicit CrossOriginPreflightResultCacheItem(StoredCredentialsPolicy storedCredentialsPolicy)
- : m_storedCredentialsPolicy(storedCredentialsPolicy)
- {
- }
+ static Expected<UniqueRef<CrossOriginPreflightResultCacheItem>, String> create(StoredCredentialsPolicy, const ResourceResponse&);
- WEBCORE_EXPORT bool parse(const ResourceResponse&);
- WEBCORE_EXPORT bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const;
- WEBCORE_EXPORT bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const;
- bool allowsRequest(StoredCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders) const;
+ CrossOriginPreflightResultCacheItem(MonotonicTime, StoredCredentialsPolicy, HashSet<String>&&, HashSet<String, ASCIICaseInsensitiveHash>&&);
+ Optional<String> validateMethodAndHeaders(const String& method, const HTTPHeaderMap&) const;
+ bool allowsRequest(StoredCredentialsPolicy, const String& method, const HTTPHeaderMap&) const;
+
private:
+ bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const;
+ bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const;
+
// FIXME: A better solution to holding onto the absolute expiration time might be
// to start a timer for the expiration delta that removes this from the cache when
// it fires.
@@ -75,4 +77,12 @@
HashMap<std::pair<String, URL>, std::unique_ptr<CrossOriginPreflightResultCacheItem>> m_preflightHashMap;
};
+inline CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem(MonotonicTime absoluteExpiryTime, StoredCredentialsPolicy storedCredentialsPolicy, HashSet<String>&& methods, HashSet<String, ASCIICaseInsensitiveHash>&& headers)
+ : m_absoluteExpiryTime(absoluteExpiryTime)
+ , m_storedCredentialsPolicy(storedCredentialsPolicy)
+ , m_methods(WTFMove(methods))
+ , m_headers(WTFMove(headers))
+{
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/network/HTTPParsers.h (258630 => 258631)
--- trunk/Source/WebCore/platform/network/HTTPParsers.h 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/platform/network/HTTPParsers.h 2020-03-18 14:49:37 UTC (rev 258631)
@@ -156,7 +156,7 @@
}
template<class HashType = DefaultHash<String>::Hash>
-HashSet<String, HashType> parseAccessControlAllowList(const String& string)
+Optional<HashSet<String, HashType>> parseAccessControlAllowList(const String& string)
{
HashSet<String, HashType> set;
unsigned start = 0;
@@ -172,7 +172,7 @@
if (!addToAccessControlAllowList(string, start, string.length() - 1, set))
return { };
}
- return set;
+ return WTFMove(set);
}
}
Modified: trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp (258630 => 258631)
--- trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp 2020-03-18 14:28:46 UTC (rev 258630)
+++ trunk/Source/WebCore/platform/network/ResourceResponseBase.cpp 2020-03-18 14:49:37 UTC (rev 258631)
@@ -180,7 +180,7 @@
ASSERT(response.tainting() == Tainting::Cors);
filteredResponse.setType(Type::Cors);
- auto accessControlExposeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders));
+ auto accessControlExposeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders)).valueOr(HashSet<String, ASCIICaseInsensitiveHash> { });
if (performCheck == PerformExposeAllHeadersCheck::Yes && accessControlExposeHeaderSet.contains("*"))
return filteredResponse;
@@ -446,7 +446,7 @@
case ResourceResponse::Tainting::Basic:
return;
case ResourceResponse::Tainting::Cors: {
- auto corsSafeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders));
+ auto corsSafeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders)).valueOr(HashSet<String, ASCIICaseInsensitiveHash> { });
if (corsSafeHeaderSet.contains("*"))
return;
