Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (230204 => 230205)
--- trunk/Source/WebCore/page/SecurityOrigin.cpp 2018-04-03 15:51:50 UTC (rev 230204)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp 2018-04-03 16:26:47 UTC (rev 230205)
@@ -123,10 +123,6 @@
// https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy (Editor's Draft, 17 November 2016)
static bool shouldTreatAsPotentiallyTrustworthy(const String& protocol, const String& host)
{
- // FIXME: despite the following SchemeRegistry functions using locks internally, we still
- // have a potential thread-safety issue with the strings being passed in. This is because
- // String::hash() will be called during lookup and it potentially modifies the String for
- // caching the hash.
if (SchemeRegistry::shouldTreatURLSchemeAsSecure(protocol))
return true;
@@ -552,13 +548,6 @@
return true;
}
-URL SecurityOrigin::urlWithUniqueSecurityOrigin()
-{
- ASSERT(isMainThread());
- static NeverDestroyed<URL> uniqueSecurityOriginURL(ParsedURLString, MAKE_STATIC_STRING_IMPL("data:,"));
- return uniqueSecurityOriginURL;
-}
-
bool SecurityOrigin::isLocalHostOrLoopbackIPAddress(const String& host)
{
if (isLoopbackIPAddress(host))
Modified: trunk/Source/WebCore/page/SecurityPolicy.cpp (230204 => 230205)
--- trunk/Source/WebCore/page/SecurityPolicy.cpp 2018-04-03 15:51:50 UTC (rev 230204)
+++ trunk/Source/WebCore/page/SecurityPolicy.cpp 2018-04-03 16:26:47 UTC (rev 230205)
@@ -45,8 +45,15 @@
typedef Vector<OriginAccessEntry> OriginAccessWhiteList;
typedef HashMap<String, std::unique_ptr<OriginAccessWhiteList>> OriginAccessMap;
+static Lock& originAccessMapLock()
+{
+ static NeverDestroyed<Lock> lock;
+ return lock;
+}
+
static OriginAccessMap& originAccessMap()
{
+ ASSERT(originAccessMapLock().isHeld());
static NeverDestroyed<OriginAccessMap> originAccessMap;
return originAccessMap;
}
@@ -146,6 +153,7 @@
bool SecurityPolicy::isAccessWhiteListed(const SecurityOrigin* activeOrigin, const SecurityOrigin* targetOrigin)
{
+ Locker<Lock> locker(originAccessMapLock());
if (OriginAccessWhiteList* list = originAccessMap().get(activeOrigin->toString())) {
for (auto& entry : *list) {
if (entry.matchesOrigin(*targetOrigin))
@@ -163,12 +171,13 @@
void SecurityPolicy::addOriginAccessWhitelistEntry(const SecurityOrigin& sourceOrigin, const String& destinationProtocol, const String& destinationDomain, bool allowDestinationSubdomains)
{
- ASSERT(isMainThread());
ASSERT(!sourceOrigin.isUnique());
if (sourceOrigin.isUnique())
return;
String sourceString = sourceOrigin.toString();
+
+ Locker<Lock> locker(originAccessMapLock());
OriginAccessMap::AddResult result = originAccessMap().add(sourceString, nullptr);
if (result.isNewEntry)
result.iterator->value = std::make_unique<OriginAccessWhiteList>();
@@ -179,12 +188,13 @@
void SecurityPolicy::removeOriginAccessWhitelistEntry(const SecurityOrigin& sourceOrigin, const String& destinationProtocol, const String& destinationDomain, bool allowDestinationSubdomains)
{
- ASSERT(isMainThread());
ASSERT(!sourceOrigin.isUnique());
if (sourceOrigin.isUnique())
return;
String sourceString = sourceOrigin.toString();
+
+ Locker<Lock> locker(originAccessMapLock());
OriginAccessMap& map = originAccessMap();
OriginAccessMap::iterator it = map.find(sourceString);
if (it == map.end())
@@ -201,7 +211,7 @@
void SecurityPolicy::resetOriginAccessWhitelists()
{
- ASSERT(isMainThread());
+ Locker<Lock> locker(originAccessMapLock());
originAccessMap().clear();
}
Modified: trunk/Source/WebCore/platform/SchemeRegistry.cpp (230204 => 230205)
--- trunk/Source/WebCore/platform/SchemeRegistry.cpp 2018-04-03 15:51:50 UTC (rev 230204)
+++ trunk/Source/WebCore/platform/SchemeRegistry.cpp 2018-04-03 16:26:47 UTC (rev 230205)
@@ -65,6 +65,12 @@
return set;
}
+static Lock& schemeRegistryLock()
+{
+ static NeverDestroyed<Lock> lock;
+ return lock;
+}
+
static const URLSchemesMap& allBuiltinSchemes()
{
static const auto schemes = makeNeverDestroyed([] {
@@ -91,10 +97,14 @@
};
URLSchemesMap set;
- for (auto& scheme : builtinLocalURLSchemes())
- set.add(scheme);
- for (auto& function : functions)
- add(set, function);
+ {
+ Locker<Lock> locker(schemeRegistryLock());
+ for (auto& scheme : builtinLocalURLSchemes())
+ set.add(scheme);
+
+ for (auto& function : functions)
+ add(set, function);
+ }
for (auto& scheme : otherSchemes)
set.add(scheme);
return set;
@@ -104,6 +114,7 @@
static const URLSchemesMap& builtinLocalURLSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static const auto schemes = makeNeverDestroyed(URLSchemesMap {
"file",
#if PLATFORM(COCOA)
@@ -113,15 +124,9 @@
return schemes;
}
-static Lock& localURLSchemesLock()
-{
- static NeverDestroyed<Lock> lock;
- return lock;
-}
-
static URLSchemesMap& localURLSchemes()
{
- ASSERT(localURLSchemesLock().isHeld());
+ ASSERT(schemeRegistryLock().isHeld());
static NeverDestroyed<URLSchemesMap> localSchemes = builtinLocalURLSchemes();
return localSchemes;
}
@@ -128,6 +133,7 @@
static URLSchemesMap& displayIsolatedURLSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static NeverDestroyed<URLSchemesMap> displayIsolatedSchemes;
return displayIsolatedSchemes;
}
@@ -134,6 +140,7 @@
const Vector<String>& builtinSecureSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static const auto schemes = makeNeverDestroyed(Vector<String> {
"https",
"about",
@@ -146,15 +153,9 @@
return schemes;
}
-static Lock& secureSchemesLock()
-{
- static NeverDestroyed<Lock> lock;
- return lock;
-}
-
static URLSchemesMap& secureSchemes()
{
- ASSERT(secureSchemesLock().isHeld());
+ ASSERT(schemeRegistryLock().isHeld());
static auto secureSchemes = makeNeverDestroyedSchemeSet(builtinSecureSchemes);
return secureSchemes;
}
@@ -161,6 +162,7 @@
const Vector<String>& builtinSchemesWithUniqueOrigins()
{
+ ASSERT(schemeRegistryLock().isHeld());
static const auto schemes = makeNeverDestroyed(Vector<String> {
"about",
"_javascript_",
@@ -173,6 +175,7 @@
static URLSchemesMap& schemesWithUniqueOrigins()
{
+ ASSERT(schemeRegistryLock().isHeld());
static auto schemesWithUniqueOrigins = makeNeverDestroyedSchemeSet(builtinSchemesWithUniqueOrigins);
return schemesWithUniqueOrigins;
}
@@ -179,6 +182,7 @@
const Vector<String>& builtinEmptyDocumentSchemes()
{
+ ASSERT(isMainThread());
static const auto schemes = makeNeverDestroyed(Vector<String> { "about" });
return schemes;
}
@@ -185,6 +189,7 @@
static URLSchemesMap& emptyDocumentSchemes()
{
+ ASSERT(isMainThread());
static auto emptyDocumentSchemes = makeNeverDestroyedSchemeSet(builtinEmptyDocumentSchemes);
return emptyDocumentSchemes;
}
@@ -191,6 +196,7 @@
static URLSchemesMap& schemesForbiddenFromDomainRelaxation()
{
+ ASSERT(isMainThread());
static NeverDestroyed<URLSchemesMap> schemes;
return schemes;
}
@@ -197,6 +203,7 @@
const Vector<String>& builtinCanDisplayOnlyIfCanRequestSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static const auto schemes = makeNeverDestroyed(Vector<String> { "blob" });
return schemes;
}
@@ -203,6 +210,7 @@
static URLSchemesMap& canDisplayOnlyIfCanRequestSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static auto canDisplayOnlyIfCanRequestSchemes = makeNeverDestroyedSchemeSet(builtinCanDisplayOnlyIfCanRequestSchemes);
return canDisplayOnlyIfCanRequestSchemes;
}
@@ -209,6 +217,7 @@
static URLSchemesMap& notAllowingJavascriptURLsSchemes()
{
+ ASSERT(isMainThread());
static NeverDestroyed<URLSchemesMap> notAllowingJavascriptURLsSchemes;
return notAllowingJavascriptURLsSchemes;
}
@@ -218,13 +227,13 @@
if (scheme.isNull())
return;
- Locker<Lock> locker(localURLSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
localURLSchemes().add(scheme);
}
void SchemeRegistry::removeURLSchemeRegisteredAsLocal(const String& scheme)
{
- Locker<Lock> locker(localURLSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
if (builtinLocalURLSchemes().contains(scheme))
return;
@@ -233,6 +242,7 @@
static URLSchemesMap& schemesAllowingLocalStorageAccessInPrivateBrowsing()
{
+ ASSERT(isMainThread());
static NeverDestroyed<URLSchemesMap> schemesAllowingLocalStorageAccessInPrivateBrowsing;
return schemesAllowingLocalStorageAccessInPrivateBrowsing;
}
@@ -239,6 +249,7 @@
static URLSchemesMap& schemesAllowingDatabaseAccessInPrivateBrowsing()
{
+ ASSERT(isMainThread());
static NeverDestroyed<URLSchemesMap> schemesAllowingDatabaseAccessInPrivateBrowsing;
return schemesAllowingDatabaseAccessInPrivateBrowsing;
}
@@ -245,6 +256,7 @@
const Vector<String>& builtinCORSEnabledSchemes()
{
+ ASSERT(isMainThread());
static const auto schemes = makeNeverDestroyed(Vector<String> { "http", "https" });
return schemes;
}
@@ -251,6 +263,7 @@
static URLSchemesMap& CORSEnabledSchemes()
{
+ ASSERT(isMainThread());
// FIXME: http://bugs.webkit.org/show_bug.cgi?id=77160
static auto schemes = makeNeverDestroyedSchemeSet(builtinCORSEnabledSchemes);
return schemes;
@@ -258,6 +271,7 @@
static URLSchemesMap& ContentSecurityPolicyBypassingSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static NeverDestroyed<URLSchemesMap> schemes;
return schemes;
}
@@ -264,19 +278,14 @@
static URLSchemesMap& cachePartitioningSchemes()
{
+ ASSERT(schemeRegistryLock().isHeld());
static NeverDestroyed<URLSchemesMap> schemes;
return schemes;
}
-static Lock& serviceWorkerSchemesLock()
-{
- static NeverDestroyed<Lock> lock;
- return lock;
-}
-
static URLSchemesMap& serviceWorkerSchemes()
{
- ASSERT(serviceWorkerSchemesLock().isHeld());
+ ASSERT(schemeRegistryLock().isHeld());
static NeverDestroyed<URLSchemesMap> schemes;
return schemes;
}
@@ -283,6 +292,7 @@
static URLSchemesMap& alwaysRevalidatedSchemes()
{
+ ASSERT(isMainThread());
static NeverDestroyed<URLSchemesMap> schemes;
return schemes;
}
@@ -292,7 +302,7 @@
if (scheme.isNull())
return false;
- Locker<Lock> locker(localURLSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
return localURLSchemes().contains(scheme);
}
@@ -300,12 +310,18 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
schemesWithUniqueOrigins().add(scheme);
}
bool SchemeRegistry::shouldTreatURLSchemeAsNoAccess(const String& scheme)
{
- return !scheme.isNull() && schemesWithUniqueOrigins().contains(scheme);
+ if (scheme.isNull())
+ return false;
+
+ Locker<Lock> locker(schemeRegistryLock());
+ return schemesWithUniqueOrigins().contains(scheme);
}
void SchemeRegistry::registerURLSchemeAsDisplayIsolated(const String& scheme)
@@ -312,12 +328,18 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
displayIsolatedURLSchemes().add(scheme);
}
bool SchemeRegistry::shouldTreatURLSchemeAsDisplayIsolated(const String& scheme)
{
- return !scheme.isNull() && displayIsolatedURLSchemes().contains(scheme);
+ if (scheme.isNull())
+ return false;
+
+ Locker<Lock> locker(schemeRegistryLock());
+ return displayIsolatedURLSchemes().contains(scheme);
}
void SchemeRegistry::registerURLSchemeAsSecure(const String& scheme)
@@ -325,7 +347,7 @@
if (scheme.isNull())
return;
- Locker<Lock> locker(secureSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
secureSchemes().add(scheme);
}
@@ -334,7 +356,7 @@
if (scheme.isNull())
return false;
- Locker<Lock> locker(secureSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
return secureSchemes().contains(scheme);
}
@@ -368,7 +390,11 @@
bool SchemeRegistry::canDisplayOnlyIfCanRequest(const String& scheme)
{
- return !scheme.isNull() && canDisplayOnlyIfCanRequestSchemes().contains(scheme);
+ if (scheme.isNull())
+ return false;
+
+ Locker<Lock> locker(schemeRegistryLock());
+ return canDisplayOnlyIfCanRequestSchemes().contains(scheme);
}
void SchemeRegistry::registerAsCanDisplayOnlyIfCanRequest(const String& scheme)
@@ -375,6 +401,8 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
canDisplayOnlyIfCanRequestSchemes().add(scheme);
}
@@ -430,6 +458,8 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
ContentSecurityPolicyBypassingSchemes().add(scheme);
}
@@ -437,12 +467,18 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
ContentSecurityPolicyBypassingSchemes().remove(scheme);
}
bool SchemeRegistry::schemeShouldBypassContentSecurityPolicy(const String& scheme)
{
- return !scheme.isNull() && ContentSecurityPolicyBypassingSchemes().contains(scheme);
+ if (scheme.isNull())
+ return false;
+
+ Locker<Lock> locker(schemeRegistryLock());
+ return ContentSecurityPolicyBypassingSchemes().contains(scheme);
}
void SchemeRegistry::registerURLSchemeAsAlwaysRevalidated(const String& scheme)
@@ -461,12 +497,18 @@
{
if (scheme.isNull())
return;
+
+ Locker<Lock> locker(schemeRegistryLock());
cachePartitioningSchemes().add(scheme);
}
bool SchemeRegistry::shouldPartitionCacheForURLScheme(const String& scheme)
{
- return !scheme.isNull() && cachePartitioningSchemes().contains(scheme);
+ if (scheme.isNull())
+ return false;
+
+ Locker<Lock> locker(schemeRegistryLock());
+ return cachePartitioningSchemes().contains(scheme);
}
void SchemeRegistry::registerURLSchemeServiceWorkersCanHandle(const String& scheme)
@@ -474,7 +516,7 @@
if (scheme.isNull())
return;
- Locker<Lock> locker(serviceWorkerSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
serviceWorkerSchemes().add(scheme);
}
@@ -490,13 +532,13 @@
return true;
}
- Locker<Lock> locker(serviceWorkerSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
return serviceWorkerSchemes().contains(scheme);
}
bool SchemeRegistry::isServiceWorkerContainerCustomScheme(const String& scheme)
{
- Locker<Lock> locker(serviceWorkerSchemesLock());
+ Locker<Lock> locker(schemeRegistryLock());
return !scheme.isNull() && serviceWorkerSchemes().contains(scheme);
}
Modified: trunk/Source/WebCore/platform/SchemeRegistry.h (230204 => 230205)
--- trunk/Source/WebCore/platform/SchemeRegistry.h 2018-04-03 15:51:50 UTC (rev 230204)
+++ trunk/Source/WebCore/platform/SchemeRegistry.h 2018-04-03 16:26:47 UTC (rev 230205)
@@ -37,25 +37,25 @@
class SchemeRegistry {
public:
- WEBCORE_EXPORT static void registerURLSchemeAsLocal(const String&);
- static void removeURLSchemeRegisteredAsLocal(const String&);
+ WEBCORE_EXPORT static void registerURLSchemeAsLocal(const String&); // Thread safe.
+ static void removeURLSchemeRegisteredAsLocal(const String&); // Thread safe.
- WEBCORE_EXPORT static bool shouldTreatURLSchemeAsLocal(const String&);
+ WEBCORE_EXPORT static bool shouldTreatURLSchemeAsLocal(const String&); // Thread safe.
WEBCORE_EXPORT static bool isBuiltinScheme(const String&);
// Secure schemes do not trigger mixed content warnings. For example,
// https and data are secure schemes because they cannot be corrupted by
// active network attackers.
- WEBCORE_EXPORT static void registerURLSchemeAsSecure(const String&);
- static bool shouldTreatURLSchemeAsSecure(const String&);
+ WEBCORE_EXPORT static void registerURLSchemeAsSecure(const String&); // Thread safe.
+ static bool shouldTreatURLSchemeAsSecure(const String&); // Thread safe.
- WEBCORE_EXPORT static void registerURLSchemeAsNoAccess(const String&);
- static bool shouldTreatURLSchemeAsNoAccess(const String&);
+ WEBCORE_EXPORT static void registerURLSchemeAsNoAccess(const String&); // Thread safe.
+ static bool shouldTreatURLSchemeAsNoAccess(const String&); // Thread safe.
// Display-isolated schemes can only be displayed (in the sense of
// SecurityOrigin::canDisplay) by documents from the same scheme.
- WEBCORE_EXPORT static void registerURLSchemeAsDisplayIsolated(const String&);
- static bool shouldTreatURLSchemeAsDisplayIsolated(const String&);
+ WEBCORE_EXPORT static void registerURLSchemeAsDisplayIsolated(const String&); // Thread safe.
+ static bool shouldTreatURLSchemeAsDisplayIsolated(const String&); // Thread safe.
WEBCORE_EXPORT static void registerURLSchemeAsEmptyDocument(const String&);
WEBCORE_EXPORT static bool shouldLoadURLSchemeAsEmptyDocument(const String&);
@@ -65,8 +65,8 @@
// Such schemes should delegate to SecurityOrigin::canRequest for any URL
// passed to SecurityOrigin::canDisplay.
- static bool canDisplayOnlyIfCanRequest(const String& scheme);
- WEBCORE_EXPORT static void registerAsCanDisplayOnlyIfCanRequest(const String& scheme);
+ static bool canDisplayOnlyIfCanRequest(const String& scheme); // Thread safe.
+ WEBCORE_EXPORT static void registerAsCanDisplayOnlyIfCanRequest(const String& scheme); // Thread safe.
// Schemes against which _javascript_: URLs should not be allowed to run (stop
// bookmarklets from running on sensitive pages).
@@ -86,9 +86,9 @@
// Allow resources from some schemes to load on a page, regardless of its
// Content Security Policy.
- WEBCORE_EXPORT static void registerURLSchemeAsBypassingContentSecurityPolicy(const String& scheme);
- WEBCORE_EXPORT static void removeURLSchemeRegisteredAsBypassingContentSecurityPolicy(const String& scheme);
- static bool schemeShouldBypassContentSecurityPolicy(const String& scheme);
+ WEBCORE_EXPORT static void registerURLSchemeAsBypassingContentSecurityPolicy(const String& scheme); // Thread safe.
+ WEBCORE_EXPORT static void removeURLSchemeRegisteredAsBypassingContentSecurityPolicy(const String& scheme); // Thread safe.
+ static bool schemeShouldBypassContentSecurityPolicy(const String& scheme); // Thread safe.
// Schemes whose responses should always be revalidated.
WEBCORE_EXPORT static void registerURLSchemeAsAlwaysRevalidated(const String&);
@@ -95,13 +95,13 @@
static bool shouldAlwaysRevalidateURLScheme(const String&);
// Schemes whose requests should be partitioned in the cache
- WEBCORE_EXPORT static void registerURLSchemeAsCachePartitioned(const String& scheme);
- static bool shouldPartitionCacheForURLScheme(const String& scheme);
+ WEBCORE_EXPORT static void registerURLSchemeAsCachePartitioned(const String& scheme); // Thread safe.
+ static bool shouldPartitionCacheForURLScheme(const String& scheme); // Thread safe.
// Schemes besides http(s) that service workers are allowed to handle
- WEBCORE_EXPORT static void registerURLSchemeServiceWorkersCanHandle(const String& scheme);
- WEBCORE_EXPORT static bool canServiceWorkersHandleURLScheme(const String& scheme);
- static bool isServiceWorkerContainerCustomScheme(const String& scheme);
+ WEBCORE_EXPORT static void registerURLSchemeServiceWorkersCanHandle(const String& scheme); // Thread safe.
+ WEBCORE_EXPORT static bool canServiceWorkersHandleURLScheme(const String& scheme); // Thread safe.
+ static bool isServiceWorkerContainerCustomScheme(const String& scheme); // Thread safe.
static bool isUserExtensionScheme(const String& scheme);
};