Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a8824ae

Browse files
addaleaxcodebytere
authored andcommittedJun 18, 2020
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 <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 27c90ef commit a8824ae

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed
 

‎src/node_url.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -1487,7 +1487,7 @@ void URL::Parse(const char* input,
14871487
state = kSpecialRelativeOrAuthority;
14881488
} else if (special) {
14891489
state = kSpecialAuthoritySlashes;
1490-
} else if (p[1] == '/') {
1490+
} else if (p + 1 < end && p[1] == '/') {
14911491
state = kPathOrAuthority;
14921492
p++;
14931493
} else {
@@ -1547,7 +1547,7 @@ void URL::Parse(const char* input,
15471547
}
15481548
break;
15491549
case kSpecialRelativeOrAuthority:
1550-
if (ch == '/' && p[1] == '/') {
1550+
if (ch == '/' && p + 1 < end && p[1] == '/') {
15511551
state = kSpecialAuthorityIgnoreSlashes;
15521552
p++;
15531553
} else {
@@ -1695,7 +1695,7 @@ void URL::Parse(const char* input,
16951695
break;
16961696
case kSpecialAuthoritySlashes:
16971697
state = kSpecialAuthorityIgnoreSlashes;
1698-
if (ch == '/' && p[1] == '/') {
1698+
if (ch == '/' && p + 1 < end && p[1] == '/') {
16991699
p++;
17001700
} else {
17011701
continue;

‎test/cctest/test_url.cc

+20
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,26 @@ TEST_F(URLTest, Base3) {
8181
EXPECT_EQ(simple.path(), "/baz");
8282
}
8383

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+
84104
TEST_F(URLTest, ToFilePath) {
85105
#define T(url, path) EXPECT_EQ(path, URL(url).ToFilePath())
86106
T("http://example.org/foo/bar", "");

0 commit comments

Comments
 (0)
Please sign in to comment.