Commit e3beb78
src: avoid OOB read in URL parser
This is not a big concern, because right now, all (non-test) inputs
to the parser are `'\0'`-terminated, but we should be future-proof
here and not perform these OOB reads.
PR-URL: #33640
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>1 parent c9cf41d commit e3beb78
2 files changed
Lines changed: 23 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1487,7 +1487,7 @@ void URL::Parse(const char* input, | |||
| 1487 | 1487 | state = kSpecialRelativeOrAuthority; | |
| 1488 | 1488 | } else if (special) { | |
| 1489 | 1489 | state = kSpecialAuthoritySlashes; | |
| 1490 | - } else if (p[1] == '/') { | ||
| 1490 | + } else if (p + 1 < end && p[1] == '/') { | ||
| 1491 | 1491 | state = kPathOrAuthority; | |
| 1492 | 1492 | p++; | |
| 1493 | 1493 | } else { | |
@@ -1547,7 +1547,7 @@ void URL::Parse(const char* input, | |||
| 1547 | 1547 | } | |
| 1548 | 1548 | break; | |
| 1549 | 1549 | case kSpecialRelativeOrAuthority: | |
| 1550 | - if (ch == '/' && p[1] == '/') { | ||
| 1550 | + if (ch == '/' && p + 1 < end && p[1] == '/') { | ||
| 1551 | 1551 | state = kSpecialAuthorityIgnoreSlashes; | |
| 1552 | 1552 | p++; | |
| 1553 | 1553 | } else { | |
@@ -1695,7 +1695,7 @@ void URL::Parse(const char* input, | |||
| 1695 | 1695 | break; | |
| 1696 | 1696 | case kSpecialAuthoritySlashes: | |
| 1697 | 1697 | state = kSpecialAuthorityIgnoreSlashes; | |
| 1698 | - if (ch == '/' && p[1] == '/') { | ||
| 1698 | + if (ch == '/' && p + 1 < end && p[1] == '/') { | ||
| 1699 | 1699 | p++; | |
| 1700 | 1700 | } else { | |
| 1701 | 1701 | continue; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -81,6 +81,26 @@ TEST_F(URLTest, Base3) { | |||
| 81 | 81 | EXPECT_EQ(simple.path(), "/baz"); | |
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | + TEST_F(URLTest, TruncatedAfterProtocol) { | ||
| 85 | + char input[2] = { 'q', ':' }; | ||
| 86 | + URL simple(input, sizeof(input)); | ||
| 87 | + | ||
| 88 | + EXPECT_FALSE(simple.flags() & URL_FLAGS_FAILED); | ||
| 89 | + EXPECT_EQ(simple.protocol(), "q:"); | ||
| 90 | + EXPECT_EQ(simple.host(), ""); | ||
| 91 | + EXPECT_EQ(simple.path(), "/"); | ||
| 92 | + } | ||
| 93 | + | ||
| 94 | + TEST_F(URLTest, TruncatedAfterProtocol2) { | ||
| 95 | + char input[6] = { 'h', 't', 't', 'p', ':', '/' }; | ||
| 96 | + URL simple(input, sizeof(input)); | ||
| 97 | + | ||
| 98 | + EXPECT_TRUE(simple.flags() & URL_FLAGS_FAILED); | ||
| 99 | + EXPECT_EQ(simple.protocol(), "http:"); | ||
| 100 | + EXPECT_EQ(simple.host(), ""); | ||
| 101 | + EXPECT_EQ(simple.path(), ""); | ||
| 102 | + } | ||
| 103 | + | ||
| 84 | 104 | TEST_F(URLTest, ToFilePath) { | |
| 85 | 105 | #define T(url, path) EXPECT_EQ(path, URL(url).ToFilePath()) | |
| 86 | 106 | T("http://example.org/foo/bar", ""); | |
0 commit comments