Title: [206554] trunk
Revision
206554
Author
[email protected]
Date
2016-09-28 14:53:53 -0700 (Wed, 28 Sep 2016)

Log Message

URLParser should properly handle unexpected periods and overflows in IPv4 addresses
https://bugs.webkit.org/show_bug.cgi?id=162655

Reviewed by Geoffrey Garen.

Source/WebCore:

Covered by new API tests.

* platform/URLParser.cpp:
(WebCore::URLParser::parseIPv4Number):
(WebCore::URLParser::parseIPv4Host):
* platform/URLParser.h:

Tools:

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

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (206553 => 206554)


--- trunk/Source/WebCore/ChangeLog	2016-09-28 21:42:34 UTC (rev 206553)
+++ trunk/Source/WebCore/ChangeLog	2016-09-28 21:53:53 UTC (rev 206554)
@@ -1,3 +1,17 @@
+2016-09-28  Alex Christensen  <[email protected]>
+
+        URLParser should properly handle unexpected periods and overflows in IPv4 addresses
+        https://bugs.webkit.org/show_bug.cgi?id=162655
+
+        Reviewed by Geoffrey Garen.
+
+        Covered by new API tests.
+
+        * platform/URLParser.cpp:
+        (WebCore::URLParser::parseIPv4Number):
+        (WebCore::URLParser::parseIPv4Host):
+        * platform/URLParser.h:
+
 2016-09-28  Wenson Hsieh  <[email protected]>
 
         Some media tests are crashing due to soft-linking failures

Modified: trunk/Source/WebCore/platform/URLParser.cpp (206553 => 206554)


--- trunk/Source/WebCore/platform/URLParser.cpp	2016-09-28 21:42:34 UTC (rev 206553)
+++ trunk/Source/WebCore/platform/URLParser.cpp	2016-09-28 21:53:53 UTC (rev 206554)
@@ -1974,7 +1974,6 @@
 template<typename CharacterType>
 Optional<uint32_t> URLParser::parseIPv4Number(CodePointIterator<CharacterType>& iterator, const CodePointIterator<CharacterType>& iteratorForSyntaxViolationPosition)
 {
-    // FIXME: Check for overflow.
     enum class State : uint8_t {
         UnknownBase,
         Decimal,
@@ -1983,11 +1982,15 @@
         Hex,
     };
     State state = State::UnknownBase;
-    uint32_t value = 0;
+    Checked<uint32_t, RecordOverflow> value = 0;
+    if (!iterator.atEnd() && *iterator == '.')
+        return Nullopt;
+    bool didSeeSyntaxViolation = false;
     while (!iterator.atEnd()) {
         if (*iterator == '.') {
             ++iterator;
-            return value;
+            ASSERT(!value.hasOverflowed());
+            return value.unsafeGet();
         }
         switch (state) {
         case State::UnknownBase:
@@ -1999,7 +2002,7 @@
             state = State::Decimal;
             break;
         case State::OctalOrHex:
-            syntaxViolation(iteratorForSyntaxViolationPosition);
+            didSeeSyntaxViolation = true;
             if (*iterator == 'x' || *iterator == 'X') {
                 ++iterator;
                 state = State::Hex;
@@ -2012,27 +2015,36 @@
                 return Nullopt;
             value *= 10;
             value += *iterator - '0';
+            if (UNLIKELY(value.hasOverflowed()))
+                return Nullopt;
             ++iterator;
             break;
         case State::Octal:
-            ASSERT(m_didSeeSyntaxViolation);
+            ASSERT(didSeeSyntaxViolation);
             if (*iterator < '0' || *iterator > '7')
                 return Nullopt;
             value *= 8;
             value += *iterator - '0';
+            if (UNLIKELY(value.hasOverflowed()))
+                return Nullopt;
             ++iterator;
             break;
         case State::Hex:
-            ASSERT(m_didSeeSyntaxViolation);
+            ASSERT(didSeeSyntaxViolation);
             if (!isASCIIHexDigit(*iterator))
                 return Nullopt;
             value *= 16;
             value += toASCIIHexValue(*iterator);
+            if (UNLIKELY(value.hasOverflowed()))
+                return Nullopt;
             ++iterator;
             break;
         }
     }
-    return value;
+    if (didSeeSyntaxViolation)
+        syntaxViolation(iteratorForSyntaxViolationPosition);
+    ASSERT(!value.hasOverflowed());
+    return value.unsafeGet();
 }
 
 ALWAYS_INLINE static uint64_t pow256(size_t exponent)
@@ -2069,7 +2081,7 @@
         return Nullopt;
     for (auto item : items) {
         if (item > 255)
-            return Nullopt;
+            syntaxViolation(hostBegin);
     }
 
     if (UNLIKELY(items.size() != 4))

Modified: trunk/Tools/ChangeLog (206553 => 206554)


--- trunk/Tools/ChangeLog	2016-09-28 21:42:34 UTC (rev 206553)
+++ trunk/Tools/ChangeLog	2016-09-28 21:53:53 UTC (rev 206554)
@@ -1,3 +1,13 @@
+2016-09-28  Alex Christensen  <[email protected]>
+
+        URLParser should properly handle unexpected periods and overflows in IPv4 addresses
+        https://bugs.webkit.org/show_bug.cgi?id=162655
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebCore/URLParser.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2016-09-28  Ryan Haddad  <[email protected]>
 
         Disable flaky API test WebKit2.AutoLayoutIntegration.

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (206553 => 206554)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-28 21:42:34 UTC (rev 206553)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp	2016-09-28 21:53:53 UTC (rev 206554)
@@ -244,9 +244,15 @@
     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/"});
+    checkURL("http://256../", {"http", "", "", "256..", 0, "/", "", "", "http://256../"});
+    checkURL("http://256..", {"http", "", "", "256..", 0, "/", "", "", "http://256../"});
+    checkURL("http://127..1/", {"http", "", "", "127..1", 0, "/", "", "", "http://127..1/"});
+    checkURL("http://127.a.0.1/", {"http", "", "", "127.a.0.1", 0, "/", "", "", "http://127.a.0.1/"});
+    checkURL("http://127.0.0.1/", {"http", "", "", "127.0.0.1", 0, "/", "", "", "http://127.0.0.1/"});
+    checkURL("http://12\t7.0.0.1/", {"http", "", "", "127.0.0.1", 0, "/", "", "", "http://127.0.0.1/"});
+    checkURL("http://127.\t0.0.1/", {"http", "", "", "127.0.0.1", 0, "/", "", "", "http://127.0.0.1/"});
+    checkURL("http://./", {"http", "", "", ".", 0, "/", "", "", "http://./"});
+    checkURL("http://.", {"http", "", "", ".", 0, "/", "", "", "http://./"});
     checkURL("http://123\t.256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
     checkURL("http://123.\t256/", {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
     checkURL("notspecial:/a", {"notspecial", "", "", "", 0, "/a", "", "", "notspecial:/a"});
@@ -677,6 +683,27 @@
     checkRelativeURLDifferences("//C|#foo/bar", "file:///tmp/mock/path",
         {"file", "", "", "", 0, "/C:/", "", "foo/bar", "file:///C:/#foo/bar"},
         {"", "", "", "", 0, "", "", "", "//C|#foo/bar"});
+    checkURLDifferences("http://0xFFFFFfFF/",
+        {"http", "", "", "255.255.255.255", 0, "/", "", "", "http://255.255.255.255/"},
+        {"http", "", "", "0xffffffff", 0, "/", "", "", "http://0xffffffff/"});
+    checkURLDifferences("http://0000000000000000037777777777/",
+        {"http", "", "", "255.255.255.255", 0, "/", "", "", "http://255.255.255.255/"},
+        {"http", "", "", "0000000000000000037777777777", 0, "/", "", "", "http://0000000000000000037777777777/"});
+    checkURLDifferences("http://4294967295/",
+        {"http", "", "", "255.255.255.255", 0, "/", "", "", "http://255.255.255.255/"},
+        {"http", "", "", "4294967295", 0, "/", "", "", "http://4294967295/"});
+    checkURLDifferences("http://256/",
+        {"http", "", "", "0.0.1.0", 0, "/", "", "", "http://0.0.1.0/"},
+        {"http", "", "", "256", 0, "/", "", "", "http://256/"});
+    checkURLDifferences("http://256./",
+        {"http", "", "", "0.0.1.0", 0, "/", "", "", "http://0.0.1.0/"},
+        {"http", "", "", "256.", 0, "/", "", "", "http://256./"});
+    checkURLDifferences("http://123.256/",
+        {"http", "", "", "123.0.1.0", 0, "/", "", "", "http://123.0.1.0/"},
+        {"http", "", "", "123.256", 0, "/", "", "", "http://123.256/"});
+    checkURLDifferences("http://127.%.0.1/",
+        {"", "", "", "", 0, "", "", "", "http://127.%.0.1/"},
+        {"http", "", "", "127.%.0.1", 0, "/", "", "", "http://127.%.0.1/"});
 }
 
 TEST_F(URLParserTest, DefaultPort)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to