Move HashCountedSet to WeakHashCountedSet in StyleGeneratedImage

https://bugs.webkit.org/show_bug.cgi?id=254835 rdar://107480319 Reviewed by Chris Dumez. Generated images should use a Weak container to keep track of RenderElements so that we don't trigger UAF issues. * Source/WebCore/html/CanvasBase.cpp: (WebCore:: const): * Source/WebCore/rendering/style/StyleCanvasImage.cpp: (WebCore::StyleCanvasImage::image const): (WebCore::StyleCanvasImage::canvasChanged): (WebCore::StyleCanvasImage::canvasResized): * Source/WebCore/rendering/style/StyleCrossfadeImage.cpp: (WebCore::StyleCrossfadeImage::imageChanged): * Source/WebCore/rendering/style/StyleFilterImage.cpp: (WebCore::StyleFilterImage::imageChanged): * Source/WebCore/rendering/style/StyleGeneratedImage.cpp: (WebCore::StyleGeneratedImage::addClient): (WebCore::StyleGeneratedImage::removeClient): (WebCore::StyleGeneratedImage::hasClient const): * Source/WebCore/rendering/style/StyleGeneratedImage.h: (WebCore::StyleGeneratedImage::clients const): (WebCore::StyleGeneratedImage:: const): Deleted. * Source/WebCore/rendering/style/StyleGradientImage.cpp: (WebCore::StyleGradientImage::image const): Canonical link: https://commits.webkit.org/262469@main
WebKit · Apr 1, 2023 · 4bfd15d · 4bfd15d
1 parent 4d53eb4
commit 4bfd15d
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 21 deletions.
diff --git a/Source/WebCore/html/CanvasBase.cpp b/Source/WebCore/html/CanvasBase.cpp
@@ -240,8 +240,9 @@ HashSet<Element*> CanvasBase::cssCanvasClients() const
         if (!is<StyleCanvasImage>(observer))
             continue;
 
-        for (auto& client : downcast<StyleCanvasImage>(observer).clients().values()) {
-            if (auto element = client->element())
+        for (auto entry : downcast<StyleCanvasImage>(observer).clients()) {
+            auto& client = entry.key;
+            if (auto element = client.element())
                 cssCanvasClients.add(element);
         }
     }

diff --git a/Source/WebCore/rendering/style/StyleCanvasImage.cpp b/Source/WebCore/rendering/style/StyleCanvasImage.cpp
@@ -76,7 +76,7 @@ RefPtr<Image> StyleCanvasImage::image(const RenderElement* renderer, const Float
     if (!renderer)
         return &Image::nullImage();
 
-    ASSERT(clients().contains(const_cast<RenderElement*>(renderer)));
+    ASSERT(clients().contains(const_cast<RenderElement&>(*renderer)));
     auto* element = this->element(renderer->document());
     if (!element || !element->buffer())
         return nullptr;
@@ -117,17 +117,21 @@ void StyleCanvasImage::canvasChanged(CanvasBase& canvasBase, const std::optional
         return;
 
     auto imageChangeRect = enclosingIntRect(changedRect.value());
-    for (auto& client : clients().values())
-        client->imageChanged(static_cast<WrappedImagePtr>(this), &imageChangeRect);
+    for (auto entry : clients()) {
+        auto& client = entry.key;
+        client.imageChanged(static_cast<WrappedImagePtr>(this), &imageChangeRect);
+    }
 }
 
 void StyleCanvasImage::canvasResized(CanvasBase& canvasBase)
 {
     ASSERT_UNUSED(canvasBase, is<HTMLCanvasElement>(canvasBase));
     ASSERT_UNUSED(canvasBase, m_element == &downcast<HTMLCanvasElement>(canvasBase));
 
-    for (auto& client : clients().values())
-        client->imageChanged(static_cast<WrappedImagePtr>(this));
+    for (auto entry : clients()) {
+        auto& client = entry.key;
+        client.imageChanged(static_cast<WrappedImagePtr>(this));
+    }
 }
 
 void StyleCanvasImage::canvasDestroyed(CanvasBase& canvasBase)

diff --git a/Source/WebCore/rendering/style/StyleCrossfadeImage.cpp b/Source/WebCore/rendering/style/StyleCrossfadeImage.cpp
@@ -184,8 +184,10 @@ void StyleCrossfadeImage::imageChanged(CachedImage*, const IntRect*)
 {
     if (!m_inputImagesAreReady)
         return;
-    for (auto& client : clients().values())
-        client->imageChanged(this);
+    for (auto entry : clients()) {
+        auto& client = entry.key;
+        client.imageChanged(static_cast<WrappedImagePtr>(this));
+    }
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/style/StyleFilterImage.cpp b/Source/WebCore/rendering/style/StyleFilterImage.cpp
@@ -162,8 +162,10 @@ void StyleFilterImage::imageChanged(CachedImage*, const IntRect*)
     if (!m_inputImageIsReady)
         return;
 
-    for (auto& client : clients().values())
-        client->imageChanged(static_cast<WrappedImagePtr>(this));
+    for (auto entry : clients()) {
+        auto& client = entry.key;
+        client.imageChanged(static_cast<WrappedImagePtr>(this));
+    }
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/style/StyleGeneratedImage.cpp b/Source/WebCore/rendering/style/StyleGeneratedImage.cpp
@@ -138,29 +138,29 @@ void StyleGeneratedImage::computeIntrinsicDimensions(const RenderElement* render
 
 void StyleGeneratedImage::addClient(RenderElement& renderer)
 {
-    if (m_clients.isEmpty())
+    if (m_clients.isEmptyIgnoringNullReferences())
         ref();
 
-    m_clients.add(&renderer);
+    m_clients.add(renderer);
 
     this->didAddClient(renderer);
 }
 
 void StyleGeneratedImage::removeClient(RenderElement& renderer)
 {
-    ASSERT(m_clients.contains(&renderer));
-    if (!m_clients.remove(&renderer))
+    ASSERT(m_clients.contains(renderer));
+    if (!m_clients.remove(renderer))
         return;
 
     this->didRemoveClient(renderer);
 
-    if (m_clients.isEmpty())
+    if (m_clients.isEmptyIgnoringNullReferences())
         deref();
 }
 
 bool StyleGeneratedImage::hasClient(RenderElement& renderer) const
 {
-    return m_clients.contains(&renderer);
+    return m_clients.contains(renderer);
 }
 
 } // namespace WebCore
diff --git a/Source/WebCore/rendering/style/StyleGeneratedImage.h b/Source/WebCore/rendering/style/StyleGeneratedImage.h
@@ -26,8 +26,8 @@
 #include "FloatSize.h"
 #include "FloatSizeHash.h"
 #include "StyleImage.h"
-#include <wtf/HashCountedSet.h>
 #include <wtf/HashMap.h>
+#include <wtf/WeakHashCountedSet.h>
 
 namespace WebCore {
 
@@ -42,7 +42,7 @@ struct ResourceLoaderOptions;
 
 class StyleGeneratedImage : public StyleImage {
 public:
-    const HashCountedSet<RenderElement*>& clients() const { return m_clients; }
+    const WeakHashCountedSet<RenderElement>& clients() const { return m_clients; }
 
 protected:
     explicit StyleGeneratedImage(StyleImage::Type, bool fixedSize);
@@ -76,7 +76,7 @@ class StyleGeneratedImage : public StyleImage {
 
     FloatSize m_containerSize;
     bool m_fixedSize;
-    HashCountedSet<RenderElement*> m_clients;
+    WeakHashCountedSet<RenderElement> m_clients;
     HashMap<FloatSize, std::unique_ptr<CachedGeneratedImage>> m_images;
 };
 

diff --git a/Source/WebCore/rendering/style/StyleGradientImage.cpp b/Source/WebCore/rendering/style/StyleGradientImage.cpp
@@ -189,7 +189,7 @@ RefPtr<Image> StyleGradientImage::image(const RenderElement* renderer, const Flo
 
     bool cacheable = m_knownCacheableBarringFilter && !renderer->style().hasAppleColorFilter();
     if (cacheable) {
-        if (!clients().contains(const_cast<RenderElement*>(renderer)))
+        if (!clients().contains(const_cast<RenderElement&>(*renderer)))
             return nullptr;
         if (auto* result = const_cast<StyleGradientImage&>(*this).cachedImageForSize(size))
             return result;