Fix defective size_t overflow in GestureTapHighlighter.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2012 20:55:48 +0000 (20:55 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Mar 2012 20:55:48 +0000 (20:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=82605

Patch by Zalan Bujtas <zbujtas@gmail.com> on 2012-03-30
Reviewed by Kenneth Rohde Christiansen.

.:

* ManualTests/tap-gesture-in-iframe-with-tap-highlight-crash.html: Added.

Source/WebCore:

In pathForRenderer, the for loop has 'i < rects().size() - 1' as test expression,
where rects().size() returns with size_t.
In case of empty rect, it leads to unsigned int overflow. Overflow value makes
the associated for loop run with invalid values.
Fix it by making loop variable int and stop using size_t type in the test expression.
Also, return early, if no focus ring found.

Manual test added. Tap gesture highlighter is getting triggered by UI process.

* page/GestureTapHighlighter.cpp:

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

ChangeLog
ManualTests/tap-gesture-in-iframe-with-tap-highlight-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/GestureTapHighlighter.cpp

index ad766e0..a9fc46b 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-03-30  Zalan Bujtas  <zbujtas@gmail.com>
+
+        Fix defective size_t overflow in GestureTapHighlighter.
+        https://bugs.webkit.org/show_bug.cgi?id=82605
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        * ManualTests/tap-gesture-in-iframe-with-tap-highlight-crash.html: Added.
+
 2012-03-30  David Barr  <davidbarr@chromium.org>
 
         Split up top-level .gitignore and .gitattributes
diff --git a/ManualTests/tap-gesture-in-iframe-with-tap-highlight-crash.html b/ManualTests/tap-gesture-in-iframe-with-tap-highlight-crash.html
new file mode 100644 (file)
index 0000000..403c303
--- /dev/null
@@ -0,0 +1,12 @@
+<html>
+<body>
+    <p>This test verifies that touch gesture on an iframe does not crash when tap highlighting is on.</p>
+    <p style='color:green'>Tapping on the iframe should not crash.</p>
+    <iframe src='data:text/html,
+        <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
+        <html>
+        <body style="margin: 0px;"></body>
+        </html>'>
+    </iframe>
+</body>
+</html>
index 02bd0be..bbf47af 100644 (file)
@@ -1,3 +1,21 @@
+2012-03-30  Zalan Bujtas  <zbujtas@gmail.com>
+
+        Fix defective size_t overflow in GestureTapHighlighter.
+        https://bugs.webkit.org/show_bug.cgi?id=82605
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        In pathForRenderer, the for loop has 'i < rects().size() - 1' as test expression,
+        where rects().size() returns with size_t.
+        In case of empty rect, it leads to unsigned int overflow. Overflow value makes
+        the associated for loop run with invalid values.
+        Fix it by making loop variable int and stop using size_t type in the test expression.
+        Also, return early, if no focus ring found.
+
+        Manual test added. Tap gesture highlighter is getting triggered by UI process.
+
+        * page/GestureTapHighlighter.cpp:
+
 2012-03-30  Mark Pilgrim  <pilgrim@chromium.org>
 
         GEOLOCATION should be implemented as Page Supplement
index 85e41a4..97f5947 100644 (file)
@@ -143,12 +143,19 @@ Path pathForRenderer(RenderObject* o)
     Vector<IntRect> rects;
     o->addFocusRingRects(rects, /* acc. offset */ ownerFrameToMainFrameOffset(o));
 
+    if (rects.isEmpty())
+        return path;
+
     // The basic idea is to allow up to three different boxes in order to highlight
     // text with line breaks more nicer than using a bounding box.
 
     // Merge all center boxes (all but the first and the last).
     LayoutRect mid;
-    for (size_t i = 1; i < rects.size() - 1; ++i)
+
+    // Set the end value to integer. It ensures that no unsigned int overflow occurs
+    // in the test expression, in case of empty rects vector.
+    int end = rects.size() - 1;
+    for (int i = 1; i < end; ++i)
         mid.uniteIfNonZero(rects.at(i));
 
     Vector<LayoutRect> drawableRects;