REGRESSION (r123848): Heap-use-after-free in WebCore::CachedResource::didAddClient.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Aug 2012 16:35:14 +0000 (16:35 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Aug 2012 16:35:14 +0000 (16:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93632
-and corresponding-
<http://crbug.com/140656>

Patch by Huang Dongsung <luxtella@company100.net> on 2012-08-10
Reviewed by Antti Koivisto.

CachedCSSStyleSheet::didAddClient() calls CachedStyleSheetClient::setCSSStyleSheet
and HTMLLnkElement can be CachedStyleSheetClient.
HTMLLinkElement::setCSSStyleSheet may cause scripts to be executed, which could
destroy the HTMLLinkElement instance. After calliing
CachedStyleSheetClient::setCSSStyleSheet, using the CachedStyleSheetClient
instance can cause Heap-use-after-free.

r115625 prevents HTMLLinkElement from being destroyed during
HTMLLinkElement::setCSSStyleSheet, but r115625 doesn't prevent HTMLLinkElement
from being destroyed after HTMLLinkElement::setCSSStyleSheet.

So this patch calls CachedResource::didAddClient() before calling
setCSSStyleSheet() to make sure its client is not destroyed.

No new tests. it's covered by fast/css/cached-sheet-restore-crash.html.

* loader/cache/CachedCSSStyleSheet.cpp:
(WebCore::CachedCSSStyleSheet::didAddClient):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@125292 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp

index 5f458ea..32811e8 100644 (file)
@@ -1,3 +1,31 @@
+2012-08-10  Huang Dongsung  <luxtella@company100.net>
+
+        REGRESSION (r123848): Heap-use-after-free in WebCore::CachedResource::didAddClient.
+        https://bugs.webkit.org/show_bug.cgi?id=93632
+        -and corresponding-
+        <http://crbug.com/140656>
+
+        Reviewed by Antti Koivisto.
+
+        CachedCSSStyleSheet::didAddClient() calls CachedStyleSheetClient::setCSSStyleSheet
+        and HTMLLnkElement can be CachedStyleSheetClient.
+        HTMLLinkElement::setCSSStyleSheet may cause scripts to be executed, which could
+        destroy the HTMLLinkElement instance. After calliing
+        CachedStyleSheetClient::setCSSStyleSheet, using the CachedStyleSheetClient
+        instance can cause Heap-use-after-free.
+
+        r115625 prevents HTMLLinkElement from being destroyed during
+        HTMLLinkElement::setCSSStyleSheet, but r115625 doesn't prevent HTMLLinkElement
+        from being destroyed after HTMLLinkElement::setCSSStyleSheet.
+
+        So this patch calls CachedResource::didAddClient() before calling
+        setCSSStyleSheet() to make sure its client is not destroyed.
+
+        No new tests. it's covered by fast/css/cached-sheet-restore-crash.html.
+
+        * loader/cache/CachedCSSStyleSheet.cpp:
+        (WebCore::CachedCSSStyleSheet::didAddClient):
+
 2012-08-10  Kevin Ellis  <kevers@chromium.org>
 
         Horizontal scrollbar appears in the month-year selector of input[type=date]
index 64db333..7e44ebc 100644 (file)
@@ -59,10 +59,13 @@ CachedCSSStyleSheet::~CachedCSSStyleSheet()
 void CachedCSSStyleSheet::didAddClient(CachedResourceClient* c)
 {
     ASSERT(c->resourceClientType() == CachedStyleSheetClient::expectedType());
+    // CachedResource::didAddClient() must be before setCSSStyleSheet(),
+    // because setCSSStyleSheet() may cause scripts to be executed, which could destroy 'c' if it is an instance of HTMLLinkElement.
+    // see the comment of HTMLLinkElement::setCSSStyleSheet.
+    CachedResource::didAddClient(c);
+
     if (!isLoading())
         static_cast<CachedStyleSheetClient*>(c)->setCSSStyleSheet(m_resourceRequest.url(), m_response.url(), m_decoder->encoding().name(), this);
-
-    CachedResource::didAddClient(c);
 }
 
 void CachedCSSStyleSheet::setEncoding(const String& chs)