Changeset 206554 in webkit

Timestamp:

Sep 28, 2016, 2:53:53 PM (9 years ago)

Author:

achristensen@apple.com

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):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/URLParser.cpp (modified) (5 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-09-28  Alex Christensen  <achristensen@webkit.org>
+
+        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  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/platform/URLParser.cpp

@@ -1975 +1975 @@
 Optional<uint32_t> URLParser::parseIPv4Number(CodePointIterator<CharacterType>& iterator, const CodePointIterator<CharacterType>& iteratorForSyntaxViolationPosition)
 {
-    // FIXME: Check for overflow.
     enum class State : uint8_t {
         UnknownBase,
@@ -1984 +1983 @@
     };
     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) {
@@ -2000 +2003 @@
             break;
         case State::OctalOrHex:
-            syntaxViolation(iteratorForSyntaxViolationPosition);
+            didSeeSyntaxViolation = true;
             if (*iterator == 'x' || *iterator == 'X') {
                 ++iterator;
@@ -2013 +2016 @@
             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();
 }
 
@@ -2070 +2082 @@
     for (auto item : items) {
         if (item > 255)
-            return Nullopt;
+            syntaxViolation(hostBegin);
     }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2016-09-28  Alex Christensen  <achristensen@webkit.org>
+
+        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  <ryanhaddad@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebCore/URLParser.cpp

@@ -245 +245 @@
     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/"});
@@ -678 +684 @@
         {"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/"});
 }