Changeset 292901 in webkit

Timestamp:

Apr 14, 2022, 11:08:27 PM (4 years ago)

Author:

Martin Robinson

Message:

[GTK] AddressSanitizer: heap-buffer-overflow in WebCore::Length::ref()
​https://bugs.webkit.org/show_bug.cgi?id=237389

Reviewed by Žan Doberšek.

• platform/graphics/nicosia/NicosiaAnimation.cpp:

(Nicosia::createThreadsafeKeyFrames): Convert Length members of transform functions to
the fixed variety before they are moved to separate threads.
(Nicosia::Animation::Animation): Use the new helper.

• platform/graphics/transforms/TranslateTransformOperation.h: Added setters.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/nicosia/NicosiaAnimation.cpp (modified) (2 diffs)
• platform/graphics/transforms/TranslateTransformOperation.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-04-14  Martin Robinson  <mrobinson@webkit.org>
+
+        [GTK] AddressSanitizer: heap-buffer-overflow in WebCore::Length::ref()
+        https://bugs.webkit.org/show_bug.cgi?id=237389
+
+        Reviewed by Žan Doberšek.
+
+        * platform/graphics/nicosia/NicosiaAnimation.cpp:
+        (Nicosia::createThreadsafeKeyFrames): Convert Length members of transform functions to
+        the fixed variety before they are moved to separate threads.
+        (Nicosia::Animation::Animation): Use the new helper.
+        * platform/graphics/transforms/TranslateTransformOperation.h: Added setters.
+
 2022-04-14  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/platform/graphics/nicosia/NicosiaAnimation.cpp

@@ -22 +22 @@
 
 #include "LayoutSize.h"
+#include "TranslateTransformOperation.h"
 
 namespace Nicosia {
@@ -139 +140 @@
 }
 
+static KeyframeValueList createThreadsafeKeyFrames(const KeyframeValueList& originalKeyframes, const FloatSize& boxSize)
+{
+    if (originalKeyframes.property() != AnimatedPropertyTransform)
+        return originalKeyframes;
+
+    // Currently translation operations are the only transform operations that store a non-fixed
+    // Length. Some Lengths, in particular those for calc() operations, are not thread-safe or
+    // multiprocess safe, because they maintain indices into a shared HashMap of CalculationValues.
+    // This code converts all possible unsafe Length parameters to fixed Lengths, which are safe to
+    // use in other threads and across IPC channels.
+    KeyframeValueList keyframes = originalKeyframes;
+    for (unsigned i = 0; i < keyframes.size(); i++) {
+        const auto& transformValue = static_cast<const TransformAnimationValue&>(keyframes.at(i));
+        for (auto& operation : transformValue.value().operations()) {
+            if (is<TranslateTransformOperation>(operation)) {
+                TranslateTransformOperation* translation = static_cast<TranslateTransformOperation*>(operation.get());
+                translation->setX(Length(translation->xAsFloat(boxSize), LengthType::Fixed));
+                translation->setY(Length(translation->yAsFloat(boxSize), LengthType::Fixed));
+                translation->setZ(Length(translation->zAsFloat(), LengthType::Fixed));
+            }
+        }
+    }
+
+    return keyframes;
+}
+
 Animation::Animation(const String& name, const KeyframeValueList& keyframes, const FloatSize& boxSize, const WebCore::Animation& animation, MonotonicTime startTime, Seconds pauseTime, AnimationState state)
     : m_name(name.isSafeToSendToAnotherThread() ? name : name.isolatedCopy())
-    , m_keyframes(keyframes)
+    , m_keyframes(createThreadsafeKeyFrames(keyframes, boxSize))
     , m_boxSize(boxSize)
     , m_timingFunction(animation.timingFunction()->clone())

trunk/Source/WebCore/platform/graphics/transforms/TranslateTransformOperation.h

@@ -59 +59 @@
     Length z() const { return m_z; }
 
+    void setX(Length newX) { m_x = newX; }
+    void setY(Length newY) { m_y = newY; }
+    void setZ(Length newZ) { m_z = newZ; }
+
     OperationType primitiveType() const final { return isRepresentableIn2D() ? TRANSLATE : TRANSLATE_3D; }