FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

inspector: harden IP address validation again · nodejs/node@165342b · GitHub

/ node Public

Commit 165342b

Browse files
tniessen
authored and
juanarbol
committed
inspector: harden IP address validation again
Use inet_pton() to parse IP addresses, which restricts IP addresses to a small number of well-defined formats. In particular, octal and hexadecimal number formats are not allowed, and neither are leading zeros. Also explicitly reject 0.0.0.0/8 and ::/128 as non-routable. Refs: https://hackerone.com/reports/1710652 CVE-ID: CVE-2022-43548 PR-URL: nodejs-private/node-private#354 Reviewed-by: Michael Dawson <midawson@redhat.com> Reviewed-by: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-by: Rich Trott <rtrott@gmail.com>
1 parent 0774b64 commit 165342b

2 files changed

Lines changed: 141 additions & 16 deletions

File tree

‎src/inspector_socket.cc‎

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -162,25 +162,70 @@ static std::string TrimPort(const std::string& host) {
162162
}
163163

164164
static bool IsIPAddress(const std::string& host) {
165-
if (host.length() >= 4 && host.front() == '[' && host.back() == ']')
166-
return true;
167-
if (host.front() == '0') return false;
168-
uint_fast16_t accum = 0;
169-
uint_fast8_t quads = 0;
170-
bool empty = true;
171-
auto endOctet = [&accum, &quads, &empty](bool final = false) {
172-
return !empty && accum <= 0xff && ++quads <= 4 && final == (quads == 4) &&
173-
(empty = true) && !(accum = 0);
174-
};
175-
for (char c : host) {
176-
if (isdigit(c)) {
177-
if ((accum = (accum * 10) + (c - '0')) > 0xff) return false;
178-
empty = false;
179-
} else if (c != '.' || !endOctet()) {
165+
// TODO(tniessen): add CVEs to the following bullet points
166+
// To avoid DNS rebinding attacks, we are aware of the following requirements:
167+
// * the host name must be an IP address,
168+
// * the IP address must be routable, and
169+
// * the IP address must be formatted unambiguously.
170+
171+
// The logic below assumes that the string is null-terminated, so ensure that
172+
// we did not somehow end up with null characters within the string.
173+
if (host.find('\0') != std::string::npos) return false;
174+
175+
// All IPv6 addresses must be enclosed in square brackets, and anything
176+
// enclosed in square brackets must be an IPv6 address.
177+
if (host.length() >= 4 && host.front() == '[' && host.back() == ']') {
178+
// INET6_ADDRSTRLEN is the maximum length of the dual format (including the
179+
// terminating null character), which is the longest possible representation
180+
// of an IPv6 address: xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:ddd.ddd.ddd.ddd
181+
if (host.length() - 2 >= INET6_ADDRSTRLEN) return false;
182+
183+
// Annoyingly, libuv's implementation of inet_pton() deviates from other
184+
// implementations of the function in that it allows '%' in IPv6 addresses.
185+
if (host.find('%') != std::string::npos) return false;
186+
187+
// Parse the IPv6 address to ensure it is syntactically valid.
188+
char ipv6_str[INET6_ADDRSTRLEN];
189+
std::copy(host.begin() + 1, host.end() - 1, ipv6_str);
190+
ipv6_str[host.length()] = '\0';
191+
unsigned char ipv6[sizeof(struct in6_addr)];
192+
if (uv_inet_pton(AF_INET6, ipv6_str, ipv6) != 0) return false;
193+
194+
// The only non-routable IPv6 address is ::/128. It should not be necessary
195+
// to explicitly reject it because it will still be enclosed in square
196+
// brackets and not even macOS should make DNS requests in that case, but
197+
// history has taught us that we cannot be careful enough.
198+
// Note that RFC 4291 defines both "IPv4-Compatible IPv6 Addresses" and
199+
// "IPv4-Mapped IPv6 Addresses", which means that there are IPv6 addresses
200+
// (other than ::/128) that represent non-routable IPv4 addresses. However,
201+
// this translation assumes that the host is interpreted as an IPv6 address
202+
// in the first place, at which point DNS rebinding should not be an issue.
203+
if (std::all_of(ipv6, ipv6 + sizeof(ipv6), [](auto b) { return b == 0; })) {
180204
return false;
181205
}
206+
207+
// It is a syntactically valid and routable IPv6 address enclosed in square
208+
// brackets. No client should be able to misinterpret this.
209+
return true;
182210
}
183-
return endOctet(true);
211+
212+
// Anything not enclosed in square brackets must be an IPv4 address. It is
213+
// important here that inet_pton() accepts only the so-called dotted-decimal
214+
// notation, which is a strict subset of the so-called numbers-and-dots
215+
// notation that is allowed by inet_aton() and inet_addr(). This subset does
216+
// not allow hexadecimal or octal number formats.
217+
unsigned char ipv4[sizeof(struct in_addr)];
218+
if (uv_inet_pton(AF_INET, host.c_str(), ipv4) != 0) return false;
219+
220+
// The only strictly non-routable IPv4 address is 0.0.0.0, and macOS will make
221+
// DNS requests for this IP address, so we need to explicitly reject it. In
222+
// fact, we can safely reject all of 0.0.0.0/8 (see Section 3.2 of RFC 791 and
223+
// Section 3.2.1.3 of RFC 1122).
224+
// Note that inet_pton() stores the IPv4 address in network byte order.
225+
if (ipv4[0] == 0) return false;
226+
227+
// It is a routable IPv4 address in dotted-decimal notation.
228+
return true;
184229
}
185230

186231
// Constants for hybi-10 frame format.

‎test/cctest/test_inspector_socket.cc‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -925,6 +925,54 @@ TEST_F(InspectorSocketTest, HostIpTooManyOctetsChecked) {
925925
expect_handshake_failure();
926926
}
927927

928+
TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetStartChecked) {
929+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
930+
"Host: 08.1.1.1:9229\r\n\r\n";
931+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
932+
INVALID_HOST_IP_REQUEST.length());
933+
expect_handshake_failure();
934+
}
935+
936+
TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetMidChecked) {
937+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
938+
"Host: 1.09.1.1:9229\r\n\r\n";
939+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
940+
INVALID_HOST_IP_REQUEST.length());
941+
expect_handshake_failure();
942+
}
943+
944+
TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetEndChecked) {
945+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
946+
"Host: 1.1.1.009:9229\r\n\r\n";
947+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
948+
INVALID_HOST_IP_REQUEST.length());
949+
expect_handshake_failure();
950+
}
951+
952+
TEST_F(InspectorSocketTest, HostIpLeadingZeroStartChecked) {
953+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
954+
"Host: 01.1.1.1:9229\r\n\r\n";
955+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
956+
INVALID_HOST_IP_REQUEST.length());
957+
expect_handshake_failure();
958+
}
959+
960+
TEST_F(InspectorSocketTest, HostIpLeadingZeroMidChecked) {
961+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
962+
"Host: 1.1.001.1:9229\r\n\r\n";
963+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
964+
INVALID_HOST_IP_REQUEST.length());
965+
expect_handshake_failure();
966+
}
967+
968+
TEST_F(InspectorSocketTest, HostIpLeadingZeroEndChecked) {
969+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
970+
"Host: 1.1.1.01:9229\r\n\r\n";
971+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
972+
INVALID_HOST_IP_REQUEST.length());
973+
expect_handshake_failure();
974+
}
975+
928976
TEST_F(InspectorSocketTest, HostIPNonRoutable) {
929977
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
930978
"Host: 0.0.0.0:9229\r\n\r\n";
@@ -933,4 +981,36 @@ TEST_F(InspectorSocketTest, HostIPNonRoutable) {
933981
expect_handshake_failure();
934982
}
935983

984+
TEST_F(InspectorSocketTest, HostIPv6NonRoutable) {
985+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
986+
"Host: [::]:9229\r\n\r\n";
987+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
988+
INVALID_HOST_IP_REQUEST.length());
989+
expect_handshake_failure();
990+
}
991+
992+
TEST_F(InspectorSocketTest, HostIPv6NonRoutableDual) {
993+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
994+
"Host: [::0.0.0.0]:9229\r\n\r\n";
995+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
996+
INVALID_HOST_IP_REQUEST.length());
997+
expect_handshake_failure();
998+
}
999+
1000+
TEST_F(InspectorSocketTest, HostIPv4InSquareBrackets) {
1001+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
1002+
"Host: [127.0.0.1]:9229\r\n\r\n";
1003+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
1004+
INVALID_HOST_IP_REQUEST.length());
1005+
expect_handshake_failure();
1006+
}
1007+
1008+
TEST_F(InspectorSocketTest, HostIPv6InvalidAbbreviation) {
1009+
const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
1010+
"Host: [:::1]:9229\r\n\r\n";
1011+
send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
1012+
INVALID_HOST_IP_REQUEST.length());
1013+
expect_handshake_failure();
1014+
}
1015+
9361016
} // anonymous namespace

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL