decompose4 return value is unchecked, leading to potentially uninitialized data.
authordino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 22:47:34 +0000 (22:47 +0000)
committerdino@apple.com <dino@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 14 Jun 2016 22:47:34 +0000 (22:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158761
<rdar://problem/17526268>

Reviewed by Simon Fraser.

Source/WebCore:

WebCore::decompose4 could return early without initializing data.
I now initialize it, but I also started checking the return
value at all the call sites to make sure everything is sensible.

Test: transforms/undecomposable.html

* platform/graphics/transforms/PerspectiveTransformOperation.cpp:
(WebCore::PerspectiveTransformOperation::blend):
* platform/graphics/transforms/RotateTransformOperation.cpp:
(WebCore::RotateTransformOperation::blend):
* platform/graphics/transforms/TransformationMatrix.cpp:
(WebCore::decompose4):
(WebCore::TransformationMatrix::blend4):
* platform/graphics/transforms/TransformationMatrix.h:

LayoutTests:

* transforms/undecomposable-expected.txt: Added.
* transforms/undecomposable.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/transforms/undecomposable-expected.txt [new file with mode: 0644]
LayoutTests/transforms/undecomposable.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/transforms/PerspectiveTransformOperation.cpp
Source/WebCore/platform/graphics/transforms/RotateTransformOperation.cpp
Source/WebCore/platform/graphics/transforms/TransformationMatrix.cpp
Source/WebCore/platform/graphics/transforms/TransformationMatrix.h

index 1b4e542..fea8f10 100644 (file)
@@ -1,3 +1,14 @@
+2016-06-14  Dean Jackson  <dino@apple.com>
+
+        decompose4 return value is unchecked, leading to potentially uninitialized data.
+        https://bugs.webkit.org/show_bug.cgi?id=158761
+        <rdar://problem/17526268>
+
+        Reviewed by Simon Fraser.
+
+        * transforms/undecomposable-expected.txt: Added.
+        * transforms/undecomposable.html: Added.
+
 2016-06-14  Keith Miller  <keith_miller@apple.com>
 
         The Array species constructor watchpoints should be created the first time they are needed rather than on creation
diff --git a/LayoutTests/transforms/undecomposable-expected.txt b/LayoutTests/transforms/undecomposable-expected.txt
new file mode 100644 (file)
index 0000000..8b13789
--- /dev/null
@@ -0,0 +1 @@
+
diff --git a/LayoutTests/transforms/undecomposable.html b/LayoutTests/transforms/undecomposable.html
new file mode 100644 (file)
index 0000000..cab3bf1
--- /dev/null
@@ -0,0 +1,32 @@
+<script>
+if (window.testRunner) {
+    window.testRunner.dumpAsText();
+    window.testRunner.waitUntilDone();
+
+    window.addEventListener("load", function () {
+        document.querySelector(".box").addEventListener("animationend", function () {
+            window.testRunner.notifyDone();
+        }, false);
+    }, false);
+}
+</script>
+<style>
+.box {
+    width: 100px;
+    height: 100px;
+    background-color: blue;
+    animation-name: funky;
+    animation-duration: 100ms;
+}
+
+@keyframes funky {
+  from {
+      transform: matrix3d(1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0);
+  }
+  to {
+      transform: matrix3d(2, 0, 0, 0, 0, 2, 0, 0, 0, 0, 2, 0, 0, 0, 0, 2);
+  }
+}
+</style>
+<div class="box"></div>
+
index 19b3859..d78c7aa 100644 (file)
@@ -1,3 +1,26 @@
+2016-06-14  Dean Jackson  <dino@apple.com>
+
+        decompose4 return value is unchecked, leading to potentially uninitialized data.
+        https://bugs.webkit.org/show_bug.cgi?id=158761
+        <rdar://problem/17526268>
+
+        Reviewed by Simon Fraser.
+
+        WebCore::decompose4 could return early without initializing data.
+        I now initialize it, but I also started checking the return
+        value at all the call sites to make sure everything is sensible.
+
+        Test: transforms/undecomposable.html
+
+        * platform/graphics/transforms/PerspectiveTransformOperation.cpp:
+        (WebCore::PerspectiveTransformOperation::blend):
+        * platform/graphics/transforms/RotateTransformOperation.cpp:
+        (WebCore::RotateTransformOperation::blend):
+        * platform/graphics/transforms/TransformationMatrix.cpp:
+        (WebCore::decompose4):
+        (WebCore::TransformationMatrix::blend4):
+        * platform/graphics/transforms/TransformationMatrix.h:
+
 2016-06-14  Benjamin Poulain  <bpoulain@apple.com>
 
         Add the unprefixed version of the pseudo element ::placeholder
index a612aed..9407946 100644 (file)
@@ -60,11 +60,11 @@ Ref<TransformOperation> PerspectiveTransformOperation::blend(const TransformOper
     toT.applyPerspective(floatValueForLength(toP, 1));
     toT.blend(fromT, progress);
     TransformationMatrix::Decomposed4Type decomp;
-    toT.decompose4(decomp);
-
-    if (decomp.perspectiveZ) {
-        double val = -1.0 / decomp.perspectiveZ;
-        return PerspectiveTransformOperation::create(Length(clampToPositiveInteger(val), Fixed));
+    if (toT.decompose4(decomp)) {
+        if (decomp.perspectiveZ) {
+            double val = -1.0 / decomp.perspectiveZ;
+            return PerspectiveTransformOperation::create(Length(clampToPositiveInteger(val), Fixed));
+        }
     }
     return PerspectiveTransformOperation::create(Length(0, Fixed));
 }
index 52c3dbc..6f42a76 100644 (file)
@@ -78,7 +78,8 @@ Ref<TransformOperation> RotateTransformOperation::blend(const TransformOperation
     
     // Extract the result as a quaternion
     TransformationMatrix::Decomposed4Type decomp;
-    toT.decompose4(decomp);
+    if (!toT.decompose4(decomp))
+        return RotateTransformOperation::create(m_x, m_y, m_z, m_angle - m_angle * progress, m_type);;
     
     // Convert that to Axis/Angle form
     double x = -decomp.quaternionX;
index f97292c..f759013 100644 (file)
@@ -365,9 +365,10 @@ static bool decompose4(const TransformationMatrix::Matrix4& mat, TransformationM
         return false;
 
     int i, j;
-    for (i = 0; i < 4; i++)
+    for (i = 0; i < 4; i++) {
         for (j = 0; j < 4; j++)
             localMatrix[i][j] /= localMatrix[3][3];
+    }
 
     // perspectiveMatrix is used to solve for perspective, but it also provides
     // an easy way to test for singularity of the upper 3x3 component.
@@ -1541,8 +1542,10 @@ void TransformationMatrix::blend4(const TransformationMatrix& from, double progr
 {
     Decomposed4Type fromDecomp;
     Decomposed4Type toDecomp;
-    from.decompose4(fromDecomp);
-    decompose4(toDecomp);
+    if (!from.decompose4(fromDecomp))
+        return;
+    if (!decompose4(toDecomp))
+        return;
 
     blendFloat(fromDecomp.scaleX, toDecomp.scaleX, progress);
     blendFloat(fromDecomp.scaleY, toDecomp.scaleY, progress);
index 9c49c92..596e29b 100644 (file)
@@ -272,11 +272,23 @@ public:
     };
 
     struct Decomposed4Type {
-        double scaleX, scaleY, scaleZ;
-        double skewXY, skewXZ, skewYZ;
-        double quaternionX, quaternionY, quaternionZ, quaternionW;
-        double translateX, translateY, translateZ;
-        double perspectiveX, perspectiveY, perspectiveZ, perspectiveW;
+        double scaleX { 1 };
+        double scaleY { 1 };
+        double scaleZ { 1 };
+        double skewXY { 0 };
+        double skewXZ { 0 };
+        double skewYZ { 0 };
+        double quaternionX { 0 };
+        double quaternionY { 0 };
+        double quaternionZ { 0 };
+        double quaternionW { 0 };
+        double translateX { 0 };
+        double translateY { 0 };
+        double translateZ { 0 };
+        double perspectiveX { 0 };
+        double perspectiveY { 0 };
+        double perspectiveZ { 0 };
+        double perspectiveW { 1 };
 
         bool operator==(const Decomposed4Type& other) const
         {