transformCanLikelyUseFastPath() can read off the end of a string
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Sep 2017 18:49:00 +0000 (18:49 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Sep 2017 18:49:00 +0000 (18:49 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/transform-fast-paths-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/transform-fast-paths.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSParserFastPaths.cpp

index d3f918e..2880632 100644 (file)
@@ -1,3 +1,14 @@
+2017-09-01  Simon Fraser  <simon.fraser@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  <jlewis3@apple.com>
 
         Marked webrtc/datachannel/bufferedAmountLowThreshold.html as flaky on Mac WK1.
diff --git a/LayoutTests/fast/css/transform-fast-paths-expected.txt b/LayoutTests/fast/css/transform-fast-paths-expected.txt
new file mode 100644 (file)
index 0000000..e277136
--- /dev/null
@@ -0,0 +1 @@
+Test test should not trigger any invalid memory accesses when running under ASan.
diff --git a/LayoutTests/fast/css/transform-fast-paths.html b/LayoutTests/fast/css/transform-fast-paths.html
new file mode 100644 (file)
index 0000000..442e34c
--- /dev/null
@@ -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>
index 80ea9ec..71327d4 100644 (file)
@@ -1,3 +1,28 @@
+2017-09-01  Simon Fraser  <simon.fraser@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  <aestes@apple.com>
 
         [CG] Upstream CoreGraphics-related WebKitSystemInterface functions
index d23832c..d42f654 100644 (file)
@@ -1115,7 +1115,7 @@ static bool parseTransformNumberArguments(CharType*& pos, CharType* end, unsigne
     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 @@ static bool transformCanLikelyUseFastPath(const CharType* chars, unsigned length
             ++i;
             continue;
         }
+
         if (length - i < kShortestValidTransformStringLength)
             return false;
+        
         switch (toASCIILower(chars[i])) {
         case 't':
             // translate, translateX, translateY, translateZ, translate3d.
@@ -1266,12 +1268,11 @@ static bool transformCanLikelyUseFastPath(const CharType* chars, unsigned length
                 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);