* inspector: DNS rebinding in --inspect via invalid octal IP (bsc#1205119, CVE-2022-43548) OBS-URL: https://build.opensuse.org/package/show/devel:languages:nodejs/nodejs12?expand=0&rev=146
210 lines
9.2 KiB
Diff
210 lines
9.2 KiB
Diff
From 2b433af094fb79cf80f086038b7f36342cb6826f Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
|
|
Date: Sun, 25 Sep 2022 12:34:05 +0000
|
|
Subject: [PATCH] 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: https://github.com/nodejs-private/node-private/pull/354
|
|
Reviewed-by: Michael Dawson <midawson@redhat.com>
|
|
Reviewed-by: Rafael Gonzaga <rafael.nunu@hotmail.com>
|
|
Reviewed-by: Rich Trott <rtrott@gmail.com>
|
|
---
|
|
src/inspector_socket.cc | 78 +++++++++++++++++++++------
|
|
test/cctest/test_inspector_socket.cc | 80 ++++++++++++++++++++++++++++
|
|
2 files changed, 142 insertions(+), 16 deletions(-)
|
|
|
|
Index: node-v12.22.12/src/inspector_socket.cc
|
|
===================================================================
|
|
--- node-v12.22.12.orig/src/inspector_socket.cc
|
|
+++ node-v12.22.12/src/inspector_socket.cc
|
|
@@ -10,6 +10,7 @@
|
|
|
|
#include "openssl/sha.h" // Sha-1 hash
|
|
|
|
+#include <algorithm>
|
|
#include <cstring>
|
|
#include <map>
|
|
|
|
@@ -166,24 +167,70 @@ static std::string TrimPort(const std::s
|
|
}
|
|
|
|
static bool IsIPAddress(const std::string& host) {
|
|
- if (host.length() >= 4 && host.front() == '[' && host.back() == ']')
|
|
- return true;
|
|
- uint_fast16_t accum = 0;
|
|
- uint_fast8_t quads = 0;
|
|
- bool empty = true;
|
|
- auto endOctet = [&accum, &quads, &empty](bool final = false) {
|
|
- return !empty && accum <= 0xff && ++quads <= 4 && final == (quads == 4) &&
|
|
- (empty = true) && !(accum = 0);
|
|
- };
|
|
- for (char c : host) {
|
|
- if (isdigit(c)) {
|
|
- if ((accum = (accum * 10) + (c - '0')) > 0xff) return false;
|
|
- empty = false;
|
|
- } else if (c != '.' || !endOctet()) {
|
|
+ // TODO(tniessen): add CVEs to the following bullet points
|
|
+ // To avoid DNS rebinding attacks, we are aware of the following requirements:
|
|
+ // * the host name must be an IP address,
|
|
+ // * the IP address must be routable, and
|
|
+ // * the IP address must be formatted unambiguously.
|
|
+
|
|
+ // The logic below assumes that the string is null-terminated, so ensure that
|
|
+ // we did not somehow end up with null characters within the string.
|
|
+ if (host.find('\0') != std::string::npos) return false;
|
|
+
|
|
+ // All IPv6 addresses must be enclosed in square brackets, and anything
|
|
+ // enclosed in square brackets must be an IPv6 address.
|
|
+ if (host.length() >= 4 && host.front() == '[' && host.back() == ']') {
|
|
+ // INET6_ADDRSTRLEN is the maximum length of the dual format (including the
|
|
+ // terminating null character), which is the longest possible representation
|
|
+ // of an IPv6 address: xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:ddd.ddd.ddd.ddd
|
|
+ if (host.length() - 2 >= INET6_ADDRSTRLEN) return false;
|
|
+
|
|
+ // Annoyingly, libuv's implementation of inet_pton() deviates from other
|
|
+ // implementations of the function in that it allows '%' in IPv6 addresses.
|
|
+ if (host.find('%') != std::string::npos) return false;
|
|
+
|
|
+ // Parse the IPv6 address to ensure it is syntactically valid.
|
|
+ char ipv6_str[INET6_ADDRSTRLEN];
|
|
+ std::copy(host.begin() + 1, host.end() - 1, ipv6_str);
|
|
+ ipv6_str[host.length()] = '\0';
|
|
+ unsigned char ipv6[sizeof(struct in6_addr)];
|
|
+ if (uv_inet_pton(AF_INET6, ipv6_str, ipv6) != 0) return false;
|
|
+
|
|
+ // The only non-routable IPv6 address is ::/128. It should not be necessary
|
|
+ // to explicitly reject it because it will still be enclosed in square
|
|
+ // brackets and not even macOS should make DNS requests in that case, but
|
|
+ // history has taught us that we cannot be careful enough.
|
|
+ // Note that RFC 4291 defines both "IPv4-Compatible IPv6 Addresses" and
|
|
+ // "IPv4-Mapped IPv6 Addresses", which means that there are IPv6 addresses
|
|
+ // (other than ::/128) that represent non-routable IPv4 addresses. However,
|
|
+ // this translation assumes that the host is interpreted as an IPv6 address
|
|
+ // in the first place, at which point DNS rebinding should not be an issue.
|
|
+ if (std::all_of(ipv6, ipv6 + sizeof(ipv6), [](auto b) { return b == 0; })) {
|
|
return false;
|
|
}
|
|
+
|
|
+ // It is a syntactically valid and routable IPv6 address enclosed in square
|
|
+ // brackets. No client should be able to misinterpret this.
|
|
+ return true;
|
|
}
|
|
- return endOctet(true);
|
|
+
|
|
+ // Anything not enclosed in square brackets must be an IPv4 address. It is
|
|
+ // important here that inet_pton() accepts only the so-called dotted-decimal
|
|
+ // notation, which is a strict subset of the so-called numbers-and-dots
|
|
+ // notation that is allowed by inet_aton() and inet_addr(). This subset does
|
|
+ // not allow hexadecimal or octal number formats.
|
|
+ unsigned char ipv4[sizeof(struct in_addr)];
|
|
+ if (uv_inet_pton(AF_INET, host.c_str(), ipv4) != 0) return false;
|
|
+
|
|
+ // The only strictly non-routable IPv4 address is 0.0.0.0, and macOS will make
|
|
+ // DNS requests for this IP address, so we need to explicitly reject it. In
|
|
+ // fact, we can safely reject all of 0.0.0.0/8 (see Section 3.2 of RFC 791 and
|
|
+ // Section 3.2.1.3 of RFC 1122).
|
|
+ // Note that inet_pton() stores the IPv4 address in network byte order.
|
|
+ if (ipv4[0] == 0) return false;
|
|
+
|
|
+ // It is a routable IPv4 address in dotted-decimal notation.
|
|
+ return true;
|
|
}
|
|
|
|
// Constants for hybi-10 frame format.
|
|
Index: node-v12.22.12/test/cctest/test_inspector_socket.cc
|
|
===================================================================
|
|
--- node-v12.22.12.orig/test/cctest/test_inspector_socket.cc
|
|
+++ node-v12.22.12/test/cctest/test_inspector_socket.cc
|
|
@@ -925,4 +925,85 @@ TEST_F(InspectorSocketTest, HostIpTooMan
|
|
expect_handshake_failure();
|
|
}
|
|
|
|
+TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetStartChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 08.1.1.1:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetMidChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 1.09.1.1:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIpInvalidOctalOctetEndChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 1.1.1.009:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIpLeadingZeroStartChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 01.1.1.1:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIpLeadingZeroMidChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 1.1.001.1:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIpLeadingZeroEndChecked) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: 1.1.1.01:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIPv6NonRoutable) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: [::]:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIPv6NonRoutableDual) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: [::0.0.0.0]:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIPv4InSquareBrackets) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: [127.0.0.1]:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
+TEST_F(InspectorSocketTest, HostIPv6InvalidAbbreviation) {
|
|
+ const std::string INVALID_HOST_IP_REQUEST = "GET /json HTTP/1.1\r\n"
|
|
+ "Host: [:::1]:9229\r\n\r\n";
|
|
+ send_in_chunks(INVALID_HOST_IP_REQUEST.c_str(),
|
|
+ INVALID_HOST_IP_REQUEST.length());
|
|
+ expect_handshake_failure();
|
|
+}
|
|
+
|
|
} // anonymous namespace
|