From e3beb781e02769131a67b28a3a871981463c4400 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 29 May 2020 17:56:58 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/33640 Reviewed-By: Sam Roberts Reviewed-By: Gus Caplan Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig --- src/node_url.cc | 6 +++--- test/cctest/test_url.cc | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/node_url.cc b/src/node_url.cc index 27e89e8d9b7652..029a04a429b7a5 100644 --- a/src/node_url.cc +++ b/src/node_url.cc @@ -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 { @@ -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 { @@ -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; diff --git a/test/cctest/test_url.cc b/test/cctest/test_url.cc index 96f9741386360f..2e78b24a5e3424 100644 --- a/test/cctest/test_url.cc +++ b/test/cctest/test_url.cc @@ -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", "");