Changeset 195965 in webkit

Timestamp:

Feb 1, 2016 10:15:25 AM (8 years ago)

Author:

jer.noble@apple.com

Message:

REGRESSION(r195770): Use-after-free in ResourceLoaderOptions::cachingPolicy
​https://bugs.webkit.org/show_bug.cgi?id=153727
<rdar://problem/24429886>

Reviewed by Chris Dumez.

The this object may be freed after calling deleteIfPossible(). Make the early-return-if-
deleted more explicit, and only check allowsCaching() after the deleteIfPossible() return
value check.

• loader/cache/CachedResource.cpp:

(WebCore::CachedResource::removeClient):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedResource.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-02-01  Jer Noble  <jer.noble@apple.com>
+
+        REGRESSION(r195770): Use-after-free in ResourceLoaderOptions::cachingPolicy
+        https://bugs.webkit.org/show_bug.cgi?id=153727
+        <rdar://problem/24429886>
+
+        Reviewed by Chris Dumez.
+
+        The `this` object may be freed after calling deleteIfPossible(). Make the early-return-if-
+        deleted more explicit, and only check allowsCaching() after the deleteIfPossible() return
+        value check.
+
+        * loader/cache/CachedResource.cpp:
+        (WebCore::CachedResource::removeClient):
+
 2016-02-01  Dan Bernstein  <mitz@apple.com>
 

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

@@ -483 +483 @@
     }
 
-    bool deleted = deleteIfPossible();
-    if (allowsCaching() && !deleted && !hasClients()) {
-        auto& memoryCache = MemoryCache::singleton();
-        if (inCache()) {
-            memoryCache.removeFromLiveResourcesSize(*this);
-            memoryCache.removeFromLiveDecodedResourcesList(*this);
-        }
-        if (!m_switchingClientsToRevalidatedResource)
-            allClientsRemoved();
-        destroyDecodedDataIfNeeded();
-        if (response().cacheControlContainsNoStore() && url().protocolIs("https")) {
-            // RFC2616 14.9.2:
-            // "no-store: ... MUST make a best-effort attempt to remove the information from volatile storage as promptly as possible"
-            // "... History buffers MAY store such responses as part of their normal operation."
-            // We allow non-secure content to be reused in history, but we do not allow secure content to be reused.
-            memoryCache.remove(*this);
-        }
-        memoryCache.pruneSoon();
-    }
-    // This object may be dead here.
+    if (deleteIfPossible()) {
+        // `this` object is dead here.
+        return;
+    }
+
+    if (!allowsCaching() || hasClients())
+        return;
+
+    auto& memoryCache = MemoryCache::singleton();
+    if (inCache()) {
+        memoryCache.removeFromLiveResourcesSize(*this);
+        memoryCache.removeFromLiveDecodedResourcesList(*this);
+    }
+    if (!m_switchingClientsToRevalidatedResource)
+        allClientsRemoved();
+    destroyDecodedDataIfNeeded();
+    if (response().cacheControlContainsNoStore() && url().protocolIs("https")) {
+        // RFC2616 14.9.2:
+        // "no-store: ... MUST make a best-effort attempt to remove the information from volatile storage as promptly as possible"
+        // "... History buffers MAY store such responses as part of their normal operation."
+        // We allow non-secure content to be reused in history, but we do not allow secure content to be reused.
+        memoryCache.remove(*this);
+    }
+    memoryCache.pruneSoon();
 }