Title: [230044] trunk/Source/WebCore
Revision
230044
Author
cdu...@apple.com
Date
2018-03-28 13:36:03 -0700 (Wed, 28 Mar 2018)

Log Message

Thread safety issue in IDBFactory' shouldThrowSecurityException()
https://bugs.webkit.org/show_bug.cgi?id=184064

Reviewed by Ryosuke Niwa.

shouldThrowSecurityException() gets called on a non-main thread but
it ended up using the SchemeRegistry via SecurityOrigin::canAccessDatabase()
which calls SecurityOrigin::isLocal().

Since using the SchemeRegistry from the background thread is not safe
(we recently added locks which we're trying to remove), and since SecurityOrigin
methods are often called from background threads, this patch make SecurityOrigin::isLocal()
safe to call from a background thread. To achieve this, we now query the SchemeRegistry
in the SecurityOrigin constructor instead as SecurityOrigin objects are expected to be
constructed on the main thread.

* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::SecurityOrigin):
(WebCore::SecurityOrigin::isLocal const): Deleted.
* page/SecurityOrigin.h:
(WebCore::SecurityOrigin::isLocal const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (230043 => 230044)


--- trunk/Source/WebCore/ChangeLog	2018-03-28 20:31:29 UTC (rev 230043)
+++ trunk/Source/WebCore/ChangeLog	2018-03-28 20:36:03 UTC (rev 230044)
@@ -1,3 +1,27 @@
+2018-03-28  Chris Dumez  <cdu...@apple.com>
+
+        Thread safety issue in IDBFactory' shouldThrowSecurityException()
+        https://bugs.webkit.org/show_bug.cgi?id=184064
+
+        Reviewed by Ryosuke Niwa.
+
+        shouldThrowSecurityException() gets called on a non-main thread but
+        it ended up using the SchemeRegistry via SecurityOrigin::canAccessDatabase()
+        which calls SecurityOrigin::isLocal().
+
+        Since using the SchemeRegistry from the background thread is not safe
+        (we recently added locks which we're trying to remove), and since SecurityOrigin
+        methods are often called from background threads, this patch make SecurityOrigin::isLocal()
+        safe to call from a background thread. To achieve this, we now query the SchemeRegistry
+        in the SecurityOrigin constructor instead as SecurityOrigin objects are expected to be
+        constructed on the main thread.
+
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::SecurityOrigin):
+        (WebCore::SecurityOrigin::isLocal const): Deleted.
+        * page/SecurityOrigin.h:
+        (WebCore::SecurityOrigin::isLocal const):
+
 2018-03-28  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, rolling out r230033.

Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (230043 => 230044)


--- trunk/Source/WebCore/page/SecurityOrigin.cpp	2018-03-28 20:31:29 UTC (rev 230043)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp	2018-03-28 20:36:03 UTC (rev 230044)
@@ -146,6 +146,7 @@
 
 SecurityOrigin::SecurityOrigin(const URL& url)
     : m_data(SecurityOriginData::fromURL(url))
+    , m_isLocal(SchemeRegistry::shouldTreatURLSchemeAsLocal(m_data.protocol))
 {
     // document.domain starts as m_data.host, but can be set by the DOM.
     m_domain = m_data.host;
@@ -182,6 +183,7 @@
     , m_enforcesFilePathSeparation { other->m_enforcesFilePathSeparation }
     , m_needsStorageAccessFromFileURLsQuirk { other->m_needsStorageAccessFromFileURLsQuirk }
     , m_isPotentiallyTrustworthy { other->m_isPotentiallyTrustworthy }
+    , m_isLocal { other->m_isLocal }
 {
 }
 
@@ -458,11 +460,6 @@
     m_enforcesFilePathSeparation = true;
 }
 
-bool SecurityOrigin::isLocal() const
-{
-    return SchemeRegistry::shouldTreatURLSchemeAsLocal(m_data.protocol);
-}
-
 String SecurityOrigin::toString() const
 {
     if (isUnique())

Modified: trunk/Source/WebCore/page/SecurityOrigin.h (230043 => 230044)


--- trunk/Source/WebCore/page/SecurityOrigin.h	2018-03-28 20:31:29 UTC (rev 230043)
+++ trunk/Source/WebCore/page/SecurityOrigin.h	2018-03-28 20:36:03 UTC (rev 230044)
@@ -153,7 +153,7 @@
     // The local SecurityOrigin is the most privileged SecurityOrigin.
     // The local SecurityOrigin can script any document, navigate to local
     // resources, and can set arbitrary headers on XMLHttpRequests.
-    WEBCORE_EXPORT bool isLocal() const;
+    bool isLocal() const { return m_isLocal; }
 
     // The origin is a globally unique identifier assigned when the Document is
     // created. http://www.whatwg.org/specs/web-apps/current-work/#sandboxOrigin
@@ -234,6 +234,7 @@
     bool m_enforcesFilePathSeparation { false };
     bool m_needsStorageAccessFromFileURLsQuirk { false };
     bool m_isPotentiallyTrustworthy { false };
+    bool m_isLocal { false };
 };
 
 bool shouldTreatAsPotentiallyTrustworthy(const URL&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to