net: enforce strict socket address formats in SocketAddress.parse

benchmark/net/net-socketaddress-parse.js

@@ -0,0 +1,49 @@
+'use strict';
+
+const common = require('../common.js');
+const { SocketAddress } = require('net');
+
+const inputs = {
+  'ipv4': [
+    '127.0.0.1',
+    '10.168.209.250',
+    '255.255.255.255',
+  ],
+  'ipv4-port': [
+    '127.0.0.1:80',
+    '10.168.209.250:8080',
+    '255.255.255.255:65535',
+  ],
+  'ipv6': [
+    '[::1]',
+    '[2001:db8::1]',
+    '[fe80::1ff:fe23:4567:890a]',
+  ],
+  'ipv6-port': [
+    '[::1]:80',
+    '[2001:db8::1]:8080',
+    '[::ffff:127.0.0.1]:65535',
+  ],
+  'invalid': [
+    'localhost',
+    '0x7f.1',
+    '127.0.0.1:99999',
+    '[::1',
+  ],
+};
+
+const bench = common.createBenchmark(main, {
+  n: [1e6],
+  input: Object.keys(inputs),
+});
+
+function main({ n, input }) {
+  const values = inputs[input];
+  const length = values.length;
+
+  bench.start();
+  for (let i = 0; i < n; i++) {
+    SocketAddress.parse(values[i % length]);
+  }
+  bench.end(n);
+}

doc/api/net.md

@@ -458,13 +458,40 @@ added:
 added:
   - v23.4.0
   - v22.13.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/64832
+    description: Input is now parsed strictly. URL syntax, IPv6 zone ids and
+                 legacy IPv4 formats such as octal, hexadecimal and shorthand
+                 notation are no longer accepted.
 -->
 
 * `input` {string} An input string containing an IP address and optional port,
   e.g. `123.1.2.3:1234` or `[1::1]:1234`.
 * Returns: {net.SocketAddress} Returns a `SocketAddress` if parsing was successful.
   Otherwise returns `undefined`.
 
+The entire input must match one of the following forms:
+
+```text
+socket-address      = ipv4-socket-address / ipv6-socket-address
+ipv4-socket-address = ipv4-address [ ":" port ]
+ipv6-socket-address = "[" ipv6-address "]" [ ":" port ]
+
+ipv4-address        = octet 3( "." octet )
+octet               = 1*3DIGIT    ; no leading zeros; value <= 255
+ipv6-address        = RFC 4291 textual form: groups of 1*4HEXDIG (leading
+                      zeros allowed, case-insensitive), "::" compression, and
+                      an optional trailing embedded ipv4-address
+port                = 1*DIGIT     ; leading zeros allowed; value <= 65535
+```
+
+Anything else returns `undefined`, including URL components such as userinfo,
+paths, queries and fragments, surrounding whitespace, host names, non-ASCII
+digits, and the legacy IPv4 notations that permit octal (`0177.0.0.1`),
+hexadecimal (`0x7f.0.0.1`), integer (`2130706433`) and shorthand (`127.1`)
+addresses. IPv6 zone ids such as `%2` or `%eth0` are not accepted.
+
 ## Class: `net.Server`
 
 <!-- YAML

lib/internal/socketaddress.js

@@ -7,6 +7,7 @@
 
 const {
   SocketAddress: _SocketAddress,
+  parseSocketAddress,
   AF_INET,
   AF_INET6,
 } = internalBinding('block_list');
@@ -37,8 +38,6 @@
   kDeserialize,
 } = require('internal/worker/js_transferable');
 
-const { URLParse } = require('internal/url');
-
 const kHandle = Symbol('kHandle');
 const kDetail = Symbol('kDetail');
 
@@ -149,26 +148,9 @@
    */
   static parse(input) {
     validateString(input, 'input');
-    // While URL.parse is not expected to throw, there are several
-    // other pieces here that do... the destucturing, the SocketAddress
-    // constructor, etc. So we wrap this in a try/catch to be safe.
-    try {
-      const {
-        hostname: address,
-        port,
-      } = URLParse(`http://${input}`);
-      if (address[0] === '[' && address[address.length - 1] === ']') {
-        return new SocketAddress({
-          address: address.slice(1, -1),
-          port: port | 0,
-          family: 'ipv6',
-        });
-      }
-      return new SocketAddress({ address, port: port | 0 });
-    } catch {
-      // Ignore errors here. Return undefined if the input cannot
-      // be successfully parsed or is not a proper socket address.
-    }
+    const handle = parseSocketAddress(input);
+    if (handle === undefined) return undefined;
+    return new InternalSocketAddress(handle);
   }
 }
 

node.gyp

@@ -170,6 +170,7 @@
       'src/node_shadow_realm.cc',
       'src/node_snapshotable.cc',
       'src/node_sockaddr.cc',
+      'src/node_sockaddr_parser.cc',
       'src/node_stat_watcher.cc',
       'src/node_symbols.cc',
       'src/node_task_queue.cc',
@@ -310,6 +311,7 @@
       'src/node_snapshot_builder.h',
       'src/node_sockaddr.h',
       'src/node_sockaddr-inl.h',
+      'src/node_sockaddr_parser.h',
       'src/node_stat_watcher.h',
       'src/node_union_bytes.h',
       'src/node_url.h',

src/node_sockaddr.cc

@@ -7,13 +7,16 @@
 #include "node_errors.h"
 #include "node_hash.h"
 #include "node_sockaddr-inl.h"  // NOLINT(build/include_inline)
+#include "node_sockaddr_parser.h"
 #include "uv.h"
 #include "v8-fast-api-calls.h"
 
 #include <cstring>
 
 #include <memory>
+#include <optional>
 #include <string>
+#include <string_view>
 #include <vector>
 
 namespace node {
@@ -74,6 +77,19 @@ bool SocketAddress::New(int32_t family,
       family, host, port, reinterpret_cast<sockaddr_storage*>(addr->storage()));
 }
 
+bool SocketAddress::Parse(std::string_view input, SocketAddress* addr) {
+  std::optional<sockaddr_parser::parse_result> parsed =
+      sockaddr_parser::ParseSocketAddress(input);
+  if (!parsed.has_value()) return false;
+
+  CHECK_LE(parsed->host.size(), sockaddr_parser::kMaxHostLength);
+
+  char host[sockaddr_parser::kMaxHostLength + 1];
+  host[parsed->host.copy(host, parsed->host.size())] = '\0';
+
+  return New(parsed->is_ipv6 ? AF_INET6 : AF_INET, host, parsed->port, addr);
+}
+
 size_t SocketAddress::Hash::operator()(const SocketAddress& addr) const {
   // Hash only the meaningful bytes (family + port + address), not the
   // full 128-byte sockaddr_storage.
@@ -1209,6 +1225,8 @@ void SocketAddressBase::Initialize(Environment* env, Local<Object> target) {
                          "SocketAddress",
                          GetConstructorTemplate(env),
                          SetConstructorFunctionFlag::NONE);
+
+  SetMethod(env->context(), target, "parseSocketAddress", Parse);
 }
 
 BaseObjectPtr<SocketAddressBase> SocketAddressBase::Create(
@@ -1247,6 +1265,20 @@ void SocketAddressBase::New(const FunctionCallbackInfo<Value>& args) {
   new SocketAddressBase(env, args.This(), std::move(addr));
 }
 
+void SocketAddressBase::Parse(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  CHECK(args[0]->IsString());  // input
+
+  Utf8Value input(env->isolate(), args[0]);
+
+  auto addr = std::make_shared<SocketAddress>();
+  if (!SocketAddress::Parse(input.ToStringView(), addr.get())) return;
+
+  BaseObjectPtr<SocketAddressBase> base =
+      SocketAddressBase::Create(env, std::move(addr));
+  if (base) args.GetReturnValue().Set(base->object());
+}
+
 void SocketAddressBase::Detail(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   CHECK(args[0]->IsObject());

src/node_sockaddr.h

@@ -16,6 +16,7 @@
 #include <list>
 #include <memory>
 #include <string>
+#include <string_view>
 #include <unordered_map>
 
 namespace node {
@@ -69,6 +70,9 @@ class SocketAddress : public MemoryRetainer {
 
   static bool New(const char* host, uint32_t port, SocketAddress* addr);
 
+  // Returns true if parsing input as an "ip[:port]" socket address succeeded.
+  static bool Parse(std::string_view input, SocketAddress* addr);
+
   // Returns the port for an IPv4 or IPv6 address.
   inline static int GetPort(const sockaddr* addr);
   inline static int GetPort(const sockaddr_storage* addr);
@@ -173,6 +177,7 @@ class SocketAddressBase : public BaseObject {
       Environment* env, std::shared_ptr<SocketAddress> address);
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void Parse(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Detail(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void LegacyDetail(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetFlowLabel(const v8::FunctionCallbackInfo<v8::Value>& args);

src/node_sockaddr_parser.cc

@@ -0,0 +1,136 @@
+#include "node_sockaddr_parser.h"
+
+namespace node::sockaddr_parser {
+
+namespace {
+
+constexpr uint32_t kMaxPort = 65535;
+
+// Not std::isdigit: that one is locale dependent.
+std::optional<uint32_t> ToDigit(char c) {
+  if (c < '0' || c > '9') return std::nullopt;
+  return static_cast<uint32_t>(c - '0');
+}
+
+// A cursor over the input. A Read that fails consumes nothing.
+class Parser {
+ public:
+  explicit Parser(std::string_view input) : remaining_(input) {}
+
+  bool done() const { return remaining_.empty(); }
+
+  bool ReadChar(char expected) {
+    if (remaining_.empty() || remaining_.front() != expected) return false;
+    remaining_.remove_prefix(1);
+    return true;
+  }
+
+  std::optional<uint32_t> ReadNumber(uint32_t max_value);
+  std::optional<uint32_t> ReadTaggedNumber(char tag, uint32_t max_value);
+  std::optional<std::string_view> ReadHost(std::string_view delimiters);
+
+  std::optional<parse_result> ReadIPv4SocketAddress();
+  std::optional<parse_result> ReadIPv6SocketAddress();
+  std::optional<parse_result> ReadSocketAddress();
+
+ private:
+  std::string_view remaining_;
+};
+
+std::optional<uint32_t> Parser::ReadNumber(uint32_t max_value) {
+  const std::string_view start = remaining_;
+
+  uint64_t value = 0;
+  size_t digits = 0;
+
+  while (!remaining_.empty()) {
+    std::optional<uint32_t> digit = ToDigit(remaining_.front());
+    if (!digit.has_value()) break;
+    remaining_.remove_prefix(1);
+    value = value * 10 + digit.value();
+    digits++;
+    if (value > max_value) break;
+  }
+
+  if (digits == 0 || value > max_value) {
+    remaining_ = start;
+    return std::nullopt;
+  }
+
+  return static_cast<uint32_t>(value);
+}
+
+std::optional<uint32_t> Parser::ReadTaggedNumber(char tag, uint32_t max_value) {
+  const std::string_view start = remaining_;
+
+  if (!ReadChar(tag)) return std::nullopt;
+
+  std::optional<uint32_t> value = ReadNumber(max_value);
+  if (!value.has_value()) remaining_ = start;
+
+  return value;
+}
+
+std::optional<std::string_view> Parser::ReadHost(std::string_view delimiters) {
+  const size_t end = remaining_.find_first_of(delimiters);
+  const std::string_view host =
+      end == std::string_view::npos ? remaining_ : remaining_.substr(0, end);
+
+  // uv_inet_pton reads only up to the first NUL.
+  if (host.size() > kMaxHostLength ||
+      host.find('\0') != std::string_view::npos) {
+    return std::nullopt;
+  }
+
+  remaining_.remove_prefix(host.size());
+
+  return host;
+}
+
+std::optional<parse_result> Parser::ReadIPv4SocketAddress() {
+  std::optional<std::string_view> host = ReadHost(":");
+  if (!host.has_value()) return std::nullopt;
+
+  parse_result result = {};
+  result.host = *host;
+  result.port =
+      static_cast<uint16_t>(ReadTaggedNumber(':', kMaxPort).value_or(0));
+
+  return result;
+}
+
+std::optional<parse_result> Parser::ReadIPv6SocketAddress() {
+  if (!ReadChar('[')) return std::nullopt;
+
+  // '%' is a delimiter so that a zone id can never reach uv_ip6_addr, which
+  // would resolve it with if_nametoindex. ReadChar(']') below rejects it.
+  std::optional<std::string_view> host = ReadHost("%]");
+  if (!host.has_value()) return std::nullopt;
+
+  parse_result result = {};
+  result.is_ipv6 = true;
+  result.host = *host;
+
+  if (!ReadChar(']')) return std::nullopt;
+
+  result.port =
+      static_cast<uint16_t>(ReadTaggedNumber(':', kMaxPort).value_or(0));
+
+  return result;
+}
+
+std::optional<parse_result> Parser::ReadSocketAddress() {
+  return remaining_.starts_with('[') ? ReadIPv6SocketAddress()
+                                     : ReadIPv4SocketAddress();
+}
+
+}  // namespace
+
+std::optional<parse_result> ParseSocketAddress(std::string_view input) {
+  Parser parser(input);
+  std::optional<parse_result> result = parser.ReadSocketAddress();
+  if (!result.has_value() || !parser.done()) return std::nullopt;
+  return result;
+}
+
+}  // namespace node::sockaddr_parser