Skip to content

Commit

Permalink
src: avoid OOB read in URL parser
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and codebytere committed Jul 8, 2020
1 parent c9cf41d commit e3beb78
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/node_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1487,7 +1487,7 @@ void URL::Parse(const char* input,
state = kSpecialRelativeOrAuthority;
} else if (special) {
state = kSpecialAuthoritySlashes;
} else if (p[1] == '/') {
} else if (p + 1 < end && p[1] == '/') {
state = kPathOrAuthority;
p++;
} else {
Expand Down Expand Up @@ -1547,7 +1547,7 @@ void URL::Parse(const char* input,
}
break;
case kSpecialRelativeOrAuthority:
if (ch == '/' && p[1] == '/') {
if (ch == '/' && p + 1 < end && p[1] == '/') {
state = kSpecialAuthorityIgnoreSlashes;
p++;
} else {
Expand Down Expand Up @@ -1695,7 +1695,7 @@ void URL::Parse(const char* input,
break;
case kSpecialAuthoritySlashes:
state = kSpecialAuthorityIgnoreSlashes;
if (ch == '/' && p[1] == '/') {
if (ch == '/' && p + 1 < end && p[1] == '/') {
p++;
} else {
continue;
Expand Down
20 changes: 20 additions & 0 deletions test/cctest/test_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,26 @@ TEST_F(URLTest, Base3) {
EXPECT_EQ(simple.path(), "/baz");
}

TEST_F(URLTest, TruncatedAfterProtocol) {
char input[2] = { 'q', ':' };
URL simple(input, sizeof(input));

EXPECT_FALSE(simple.flags() & URL_FLAGS_FAILED);
EXPECT_EQ(simple.protocol(), "q:");
EXPECT_EQ(simple.host(), "");
EXPECT_EQ(simple.path(), "/");
}

TEST_F(URLTest, TruncatedAfterProtocol2) {
char input[6] = { 'h', 't', 't', 'p', ':', '/' };
URL simple(input, sizeof(input));

EXPECT_TRUE(simple.flags() & URL_FLAGS_FAILED);
EXPECT_EQ(simple.protocol(), "http:");
EXPECT_EQ(simple.host(), "");
EXPECT_EQ(simple.path(), "");
}

TEST_F(URLTest, ToFilePath) {
#define T(url, path) EXPECT_EQ(path, URL(url).ToFilePath())
T("http://example.org/foo/bar", "");
Expand Down

0 comments on commit e3beb78

Please sign in to comment.