Title: [207273] trunk
Revision
207273
Author
achristen...@apple.com
Date
2016-10-12 19:38:13 -0700 (Wed, 12 Oct 2016)

Log Message

Fix out-of-bounds reading in URLParser when parsing improperly percent-encoded values
https://bugs.webkit.org/show_bug.cgi?id=163376

Reviewed by Saam Barati.

Source/WebCore:

Covered by new API tests, which used to crash under asan.

* platform/URLParser.cpp:
(WebCore::percentDecode):
If you subtract 2 from size_t's smaller than 2, you're gonna have a bad time.

Tools:

* TestWebKitAPI/Tests/WebCore/URLParser.cpp:
(TestWebKitAPI::TEST_F):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207272 => 207273)


--- trunk/Source/WebCore/ChangeLog	2016-10-13 02:28:49 UTC (rev 207272)
+++ trunk/Source/WebCore/ChangeLog	2016-10-13 02:38:13 UTC (rev 207273)
@@ -1,5 +1,18 @@
 2016-10-12  Alex Christensen  <achristen...@webkit.org>
 
+        Fix out-of-bounds reading in URLParser when parsing improperly percent-encoded values
+        https://bugs.webkit.org/show_bug.cgi?id=163376
+
+        Reviewed by Saam Barati.
+
+        Covered by new API tests, which used to crash under asan.
+
+        * platform/URLParser.cpp:
+        (WebCore::percentDecode):
+        If you subtract 2 from size_t's smaller than 2, you're gonna have a bad time.
+
+2016-10-12  Alex Christensen  <achristen...@webkit.org>
+
         Mail needs nonspecial URLs to keep case in host and not have slash after host
         https://bugs.webkit.org/show_bug.cgi?id=163373
 

Modified: trunk/Source/WebCore/platform/URLParser.cpp (207272 => 207273)


--- trunk/Source/WebCore/platform/URLParser.cpp	2016-10-13 02:28:49 UTC (rev 207272)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2016-10-13 02:38:13 UTC (rev 207273)
@@ -2445,7 +2445,7 @@
         uint8_t byte = input[i];
         if (byte != '%')
             output.uncheckedAppend(byte);
-        else if (i < length - 2) {
+        else if (length > 2 && i < length - 2) {
             if (isASCIIHexDigit(input[i + 1]) && isASCIIHexDigit(input[i + 2])) {
                 output.uncheckedAppend(toASCIIHexValue(input[i + 1], input[i + 2]));
                 i += 2;

Modified: trunk/Tools/ChangeLog (207272 => 207273)


--- trunk/Tools/ChangeLog	2016-10-13 02:28:49 UTC (rev 207272)
+++ trunk/Tools/ChangeLog	2016-10-13 02:38:13 UTC (rev 207273)
@@ -1,3 +1,13 @@
+2016-10-12  Alex Christensen  <achristen...@webkit.org>
+
+        Fix out-of-bounds reading in URLParser when parsing improperly percent-encoded values
+        https://bugs.webkit.org/show_bug.cgi?id=163376
+
+        Reviewed by Saam Barati.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-10-11  Dean Jackson  <d...@apple.com>
 
         Add preliminary support for extended colors to WebCore::Color

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (207272 => 207273)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-10-13 02:28:49 UTC (rev 207272)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-10-13 02:38:13 UTC (rev 207273)
@@ -706,11 +706,31 @@
         {"file", "", "", "", 0, "/pAtH/", "", "", "file:///pAtH/"},
         {"file", "", "", "", 0, "pAtH/", "", "", "file://pAtH/"});
     
-    // FIXME: Fix and test incomplete percent encoded characters in the middle and end of the input string.
-    // FIXME: Fix and test percent encoded upper case characters in the host.
     checkURLDifferences("http://host%73",
         {"http", "", "", "hosts", 0, "/", "", "", "http://hosts/"},
         {"http", "", "", "host%73", 0, "/", "", "", "http://host%73/"});
+    checkURLDifferences("http://host%53",
+        {"http", "", "", "hosts", 0, "/", "", "", "http://hosts/"},
+        {"http", "", "", "host%53", 0, "/", "", "", "http://host%53/"});
+    checkURLDifferences("http://%",
+        {"", "", "", "", 0, "", "", "", "http://%"},
+        {"http", "", "", "%", 0, "/", "", "", "http://%/"});
+    checkURLDifferences("http://%7",
+        {"", "", "", "", 0, "", "", "", "http://%7"},
+        {"http", "", "", "%7", 0, "/", "", "", "http://%7/"});
+    checkURLDifferences("http://%7s",
+        {"", "", "", "", 0, "", "", "", "http://%7s"},
+        {"http", "", "", "%7s", 0, "/", "", "", "http://%7s/"});
+    checkURLDifferences("http://%73",
+        {"http", "", "", "s", 0, "/", "", "", "http://s/"},
+        {"http", "", "", "%73", 0, "/", "", "", "http://%73/"});
+    checkURLDifferences("http://abcdefg%",
+        {"", "", "", "", 0, "", "", "", "http://abcdefg%"},
+        {"http", "", "", "abcdefg%", 0, "/", "", "", "http://abcdefg%/"});
+    checkURLDifferences("http://abcd%7Xefg",
+        {"", "", "", "", 0, "", "", "", "http://abcd%7Xefg"},
+        {"http", "", "", "abcd%7xefg", 0, "/", "", "", "http://abcd%7xefg/"});
+
     
     // URLParser matches Chrome and the spec, but not URL::parse or Firefox.
     checkURLDifferences(utf16String(u"http://0Xc0.0250.01"),
@@ -1101,6 +1121,8 @@
     shouldFail("i");
     shouldFail("asdf");
     shouldFail("~");
+    shouldFail("%");
+    shouldFail("//%");
     shouldFail("~", "about:blank");
     shouldFail("~~~");
     shouldFail("://:0/");
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to