Changeset 201565 in webkit

Timestamp:

Jun 1, 2016, 1:15:23 PM (10 years ago)

Author:

commit-queue@webkit.org

Message:

Fix null dereferencing in ResourceTimingInformation::addResourceTiming
​https://bugs.webkit.org/show_bug.cgi?id=158266
rdar://problem/26528466

Patch by Alex Christensen <​achristensen@webkit.org> on 2016-06-01
Reviewed by Chris Dumez.

This would crash sometimes in http/tests/security/cross-frame-access-custom.html

• loader/ResourceTimingInformation.cpp:

(WebCore::ResourceTimingInformation::addResourceTiming):

• loader/ResourceTimingInformation.h:
• loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::requestResource):
(WebCore::CachedResourceLoader::loadDone):
We would dereference a null pointer on the first instruction when calling
Document::parentDocument from ResourceTimingInformation::addResourceTiming,
which means the document was null. CachedResourceLoader::document even has
a comment saying it can be null, so we should check it before using it.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/ResourceTimingInformation.cpp (modified) (1 diff)
• loader/ResourceTimingInformation.h (modified) (1 diff)
• loader/cache/CachedResourceLoader.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-06-01  Alex Christensen  <achristensen@webkit.org>
+
+        Fix null dereferencing in ResourceTimingInformation::addResourceTiming
+        https://bugs.webkit.org/show_bug.cgi?id=158266
+        rdar://problem/26528466
+
+        Reviewed by Chris Dumez.
+
+        This would crash sometimes in http/tests/security/cross-frame-access-custom.html
+
+        * loader/ResourceTimingInformation.cpp:
+        (WebCore::ResourceTimingInformation::addResourceTiming):
+        * loader/ResourceTimingInformation.h:
+        * loader/cache/CachedResourceLoader.cpp:
+        (WebCore::CachedResourceLoader::requestResource):
+        (WebCore::CachedResourceLoader::loadDone):
+        We would dereference a null pointer on the first instruction when calling 
+        Document::parentDocument from ResourceTimingInformation::addResourceTiming, 
+        which means the document was null.  CachedResourceLoader::document even has
+        a comment saying it can be null, so we should check it before using it.
+
 2016-06-01  Said Abou-Hallawa  <sabouhallawa@apple.com>
 

trunk/Source/WebCore/loader/ResourceTimingInformation.cpp

@@ -39 +39 @@
 namespace WebCore {
 
-void ResourceTimingInformation::addResourceTiming(CachedResource* resource, Document* document)
+void ResourceTimingInformation::addResourceTiming(CachedResource* resource, Document& document)
 {
     ASSERT(RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled());
     if (resource && resource->resourceRequest().url().protocolIsInHTTPFamily()
         && ((!resource->errorOccurred() && !resource->wasCanceled()) || resource->response().httpStatusCode() == 304)) {
-        HashMap<CachedResource*, InitiatorInfo>::iterator initiatorIt = m_initiatorMap.find(resource);
+        auto initiatorIt = m_initiatorMap.find(resource);
         if (initiatorIt != m_initiatorMap.end() && initiatorIt->value.added == NotYetAdded) {
-            ASSERT(document);
-            Document* initiatorDocument = document;
+            Document* initiatorDocument = &document;
             if (resource->type() == CachedResource::MainResource)
-                initiatorDocument = document->parentDocument();
+                initiatorDocument = document.parentDocument();
             ASSERT(initiatorDocument);
             ASSERT(initiatorDocument->domWindow());

trunk/Source/WebCore/loader/ResourceTimingInformation.h

@@ -42 +42 @@
 public:
 
-    void addResourceTiming(CachedResource*, Document*);
+    void addResourceTiming(CachedResource*, Document&);
     void storeResourceTimingInitiatorInformation(const CachedResourceHandle<CachedResource>&, const CachedResourceRequest&, Frame*);
 

trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

@@ -620 +620 @@
         memoryCache.resourceAccessed(*resource);
 #if ENABLE(WEB_TIMING)
-        if (RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled()) {
+        if (document() && RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled()) {
             m_resourceTimingInfo.storeResourceTimingInitiatorInformation(resource, request, frame());
-            m_resourceTimingInfo.addResourceTiming(resource.get(), document());
+            m_resourceTimingInfo.addResourceTiming(resource.get(), *document());
         }
 #endif
@@ -968 +968 @@
 
 #if ENABLE(WEB_TIMING)
-    if (RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled())
-        m_resourceTimingInfo.addResourceTiming(resource, document());
+    if (document() && RuntimeEnabledFeatures::sharedFeatures().resourceTimingEnabled())
+        m_resourceTimingInfo.addResourceTiming(resource, *document());
 #else
     UNUSED_PARAM(resource);