URLParser can read memory out of bounds
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2016 23:05:11 +0000 (23:05 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2016 23:05:11 +0000 (23:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162206

Reviewed by Geoff Garen.

Source/WebCore:

Covered by new API tests.
URLParser is disabled by default still.

* platform/URLParser.cpp:
(WebCore::parseIPv4Host):
If there are fewer than two numbers in an ipv4 address, we would subtract two from the Vector's size,
causing us to read memory up to std::numeric_limits<size_t>::max() - 2.  Added a bounds check and many tests.

Tools:

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

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

Source/WebCore/ChangeLog
Source/WebCore/platform/URLParser.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

index 142d658..43df3c2 100644 (file)
@@ -1,5 +1,20 @@
 2016-09-19  Alex Christensen  <achristensen@webkit.org>
 
+        URLParser can read memory out of bounds
+        https://bugs.webkit.org/show_bug.cgi?id=162206
+
+        Reviewed by Geoff Garen.
+
+        Covered by new API tests.
+        URLParser is disabled by default still.
+
+        * platform/URLParser.cpp:
+        (WebCore::parseIPv4Host):
+        If there are fewer than two numbers in an ipv4 address, we would subtract two from the Vector's size, 
+        causing us to read memory up to std::numeric_limits<size_t>::max() - 2.  Added a bounds check and many tests.
+
+2016-09-19  Alex Christensen  <achristensen@webkit.org>
+
         URLParser should parse serialized valid URLs faster than unknown input
         https://bugs.webkit.org/show_bug.cgi?id=162228
 
index 7f785e0..de0ef43 100644 (file)
@@ -1769,9 +1769,11 @@ inline static Optional<uint32_t> parseIPv4Host(CodePointIterator<CharacterType>
     }
     if (!items.size() || items.size() > 4)
         return Nullopt;
-    for (size_t i = 0; i < items.size() - 2; i++) {
-        if (items[i] > 255)
-            return Nullopt;
+    if (items.size() > 2) {
+        for (size_t i = 0; i < items.size() - 2; i++) {
+            if (items[i] > 255)
+                return Nullopt;
+        }
     }
     if (items[items.size() - 1] >= pow256(5 - items.size()))
         return Nullopt;
index 4574bb3..e136f27 100644 (file)
@@ -1,3 +1,13 @@
+2016-09-19  Alex Christensen  <achristensen@webkit.org>
+
+        URLParser can read memory out of bounds
+        https://bugs.webkit.org/show_bug.cgi?id=162206
+
+        Reviewed by Geoff Garen.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-19  Daniel Bates  <dabates@apple.com>
 
         Remove ENABLE(TEXT_AUTOSIZING) automatic text size adjustment code
index 4fc2840..9db2cb4 100644 (file)
@@ -207,6 +207,9 @@ TEST_F(URLParserTest, Basic)
     checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
     checkURL("notspecial:", {"notspecial", "", "", "", 0, "", "", "", "notspecial:"});
     checkURL("http:/a", {"http", "", "", "a", 0, "/", "", "", "http://a/"});
+    checkURL("http://256/", {"http", "", "", "256", 0, "/", "", "", "http://256/"});
+    checkURL("http://256./", {"http", "", "", "256.", 0, "/", "", "", "http://256./"});
+    checkURL("http://123.256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
     // FIXME: Fix and add a test with an invalid surrogate pair at the end with a space as the second code unit.
 
     // This disagrees with the web platform test for http://:@www.example.com but agrees with Chrome and URL::parse,
@@ -508,6 +511,21 @@ TEST_F(URLParserTest, ParserDifferences)
     checkRelativeURLDifferences("http://`{}:`{}@h/`{}?`{}", "http://doesnotmatter/",
         {"http", "`{}", "`{}", "h", 0, "/%60%7B%7D", "`{}", "", "http://%60%7B%7D:%60%7B%7D@h/%60%7B%7D?`{}"},
         {"", "", "", "", 0, "", "", "", "http://`{}:`{}@h/`{}?`{}"});
+    checkURLDifferences("http://[0:f::f::f]",
+        {"", "", "", "", 0, "" , "", "", "http://[0:f::f::f]"},
+        {"http", "", "", "[0:f::f::f]", 0, "/" , "", "", "http://[0:f::f::f]/"});
+    checkURLDifferences("http://123",
+        {"http", "", "", "0.0.0.123", 0, "/", "", "", "http://0.0.0.123/"},
+        {"http", "", "", "123", 0, "/", "", "", "http://123/"});
+    checkURLDifferences("http://123.234/",
+        {"http", "", "", "123.0.0.234", 0, "/", "", "", "http://123.0.0.234/"},
+        {"http", "", "", "123.234", 0, "/", "", "", "http://123.234/"});
+    checkURLDifferences("http://123.234.012",
+        {"http", "", "", "123.234.0.10", 0, "/", "", "", "http://123.234.0.10/"},
+        {"http", "", "", "123.234.012", 0, "/", "", "", "http://123.234.012/"});
+    checkURLDifferences("http://123.234.12",
+        {"http", "", "", "123.234.0.12", 0, "/", "", "", "http://123.234.0.12/"},
+        {"http", "", "", "123.234.12", 0, "/", "", "", "http://123.234.12/"});
 }
 
 TEST_F(URLParserTest, DefaultPort)