Title: [221488] trunk
Revision
221488
Author
simon.fra...@apple.com
Date
2017-09-01 11:49:00 -0700 (Fri, 01 Sep 2017)

Log Message

transformCanLikelyUseFastPath() can read off the end of a string
https://bugs.webkit.org/show_bug.cgi?id=176232
rdar://problem/33851237

Reviewed by Tim Horton.
Source/WebCore:

Code added in r220382 could read one byte past the end of the string when looking for the 'z'
of a rotateZ() function. The code was actually incorrect, testing for the 'z at i+6 after
already incrementing i by 6. This patch makes the code correctly detect rotateZ().

Also, rotate functions at the end of a string could be ignored because kShortestValidTransformStringLength
was too long, so set it to the length of "rotate(0)", the shortest transform function that we currently
fast-parse.

There's an implicit assumption in this code that chars is not indexed past i+kShortestValidTransformStringLength.
If the 'translate' path is taken, i is incremented by 9 (==kShortestValidTransformStringLength), but that's
OK because WTF::find() doesn't index into chars if i >= length.

Test: fast/css/transform-fast-paths.html

* css/parser/CSSParserFastPaths.cpp:
(WebCore::transformCanLikelyUseFastPath):

LayoutTests:

* fast/css/transform-fast-paths-expected.txt: Added.
* fast/css/transform-fast-paths.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (221487 => 221488)


--- trunk/LayoutTests/ChangeLog	2017-09-01 18:48:27 UTC (rev 221487)
+++ trunk/LayoutTests/ChangeLog	2017-09-01 18:49:00 UTC (rev 221488)
@@ -1,3 +1,14 @@
+2017-09-01  Simon Fraser  <simon.fra...@apple.com>
+
+        transformCanLikelyUseFastPath() can read off the end of a string
+        https://bugs.webkit.org/show_bug.cgi?id=176232
+        rdar://problem/33851237
+
+        Reviewed by Tim Horton.
+
+        * fast/css/transform-fast-paths-expected.txt: Added.
+        * fast/css/transform-fast-paths.html: Added.
+
 2017-09-01  Matt Lewis  <jlew...@apple.com>
 
         Marked webrtc/datachannel/bufferedAmountLowThreshold.html as flaky on Mac WK1.

Added: trunk/LayoutTests/fast/css/transform-fast-paths-expected.txt (0 => 221488)


--- trunk/LayoutTests/fast/css/transform-fast-paths-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/css/transform-fast-paths-expected.txt	2017-09-01 18:49:00 UTC (rev 221488)
@@ -0,0 +1 @@
+Test test should not trigger any invalid memory accesses when running under ASan.

Added: trunk/LayoutTests/fast/css/transform-fast-paths.html (0 => 221488)


--- trunk/LayoutTests/fast/css/transform-fast-paths.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/transform-fast-paths.html	2017-09-01 18:49:00 UTC (rev 221488)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<body>
+    Test test should not trigger any invalid memory accesses when running under ASan.
+
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    var heading = document.createElement("div");
+    heading.style.setProperty("transform", "rotateZ(8deg)");
+    heading.style.setProperty("transform", "rotate(0)");
+    heading.style.setProperty("transform", "translate");
+    heading.style.setProperty("transform", "translate(0px, 1px) rotate(0deg) translate(0px, 0px) rotate(0deg)");
+    heading.style.setProperty("transform", "translate(0px, 1px) rotate(0deg) translate(0px, 0px) rotate(0)");
+    heading.style.setProperty("transform", "translate(0px, 1px) rotate(0deg) translate(0px, 0px) rotate");
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (221487 => 221488)


--- trunk/Source/WebCore/ChangeLog	2017-09-01 18:48:27 UTC (rev 221487)
+++ trunk/Source/WebCore/ChangeLog	2017-09-01 18:49:00 UTC (rev 221488)
@@ -1,3 +1,28 @@
+2017-09-01  Simon Fraser  <simon.fra...@apple.com>
+
+        transformCanLikelyUseFastPath() can read off the end of a string
+        https://bugs.webkit.org/show_bug.cgi?id=176232
+        rdar://problem/33851237
+
+        Reviewed by Tim Horton.
+        
+        Code added in r220382 could read one byte past the end of the string when looking for the 'z'
+        of a rotateZ() function. The code was actually incorrect, testing for the 'z at i+6 after
+        already incrementing i by 6. This patch makes the code correctly detect rotateZ().
+        
+        Also, rotate functions at the end of a string could be ignored because kShortestValidTransformStringLength
+        was too long, so set it to the length of "rotate(0)", the shortest transform function that we currently
+        fast-parse.
+
+        There's an implicit assumption in this code that chars is not indexed past i+kShortestValidTransformStringLength.
+        If the 'translate' path is taken, i is incremented by 9 (==kShortestValidTransformStringLength), but that's
+        OK because WTF::find() doesn't index into chars if i >= length.
+
+        Test: fast/css/transform-fast-paths.html
+
+        * css/parser/CSSParserFastPaths.cpp:
+        (WebCore::transformCanLikelyUseFastPath):
+
 2017-09-01  Andy Estes  <aes...@apple.com>
 
         [CG] Upstream CoreGraphics-related WebKitSystemInterface functions

Modified: trunk/Source/WebCore/css/parser/CSSParserFastPaths.cpp (221487 => 221488)


--- trunk/Source/WebCore/css/parser/CSSParserFastPaths.cpp	2017-09-01 18:48:27 UTC (rev 221487)
+++ trunk/Source/WebCore/css/parser/CSSParserFastPaths.cpp	2017-09-01 18:49:00 UTC (rev 221488)
@@ -1115,7 +1115,7 @@
     return true;
 }
 
-static const int kShortestValidTransformStringLength = 12;
+static const int kShortestValidTransformStringLength = 9; // "rotate(0)"
 
 template <typename CharType>
 static RefPtr<CSSFunctionValue> parseSimpleTransformValue(CharType*& pos, CharType* end)
@@ -1239,8 +1239,10 @@
             ++i;
             continue;
         }
+
         if (length - i < kShortestValidTransformStringLength)
             return false;
+        
         switch (toASCIILower(chars[i])) {
         case 't':
             // translate, translateX, translateY, translateZ, translate3d.
@@ -1266,12 +1268,11 @@
                 return false;
             i += 6;
             // rotateZ
-            if (toASCIILower(chars[i + 6]) == 'z')
+            if (toASCIILower(chars[i]) == 'z')
                 ++i;
             break;
 
         default:
-            // All other things, ex. rotate.
             return false;
         }
         size_t argumentsEnd = WTF::find(chars, length, ')', i);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to