dns: refuse setServers() ports above 65535

src/js/node/dns.ts

@@ -158,6 +158,7 @@ function setServersOn(servers: readonly string[], object: { setServers(servers:
       ipVersion = isIP(match[1]);
       if (ipVersion !== 0) {
         const port = parseInt(addrSplitRE[Symbol.replace](server, "$2")) || IANA_DNS_PORT;
+        validatePort(port);
         triples.push([ipVersion, stripZoneId(match[1]), port]);
         return;
       }
@@ -168,11 +169,12 @@ function setServersOn(servers: readonly string[], object: { setServers(servers:
 
     if (addrSplitMatch) {
       const hostIP = addrSplitMatch[1];
-      const port = addrSplitMatch[2] ? parseInt(addrSplitMatch[2]) : IANA_DNS_PORT;
+      const port = parseInt(addrSplitMatch[2]) || IANA_DNS_PORT;
 
       ipVersion = isIP(hostIP);
 
       if (ipVersion !== 0) {
+        validatePort(port);
         triples.push([ipVersion, hostIP, port]);
         return;
       }

test/js/node/dns/dns-resolver-class.test.ts

@@ -1,7 +1,9 @@
 // node-dns.test.js resolves public hostnames. These tests need no network.
 import { describe, expect, test } from "bun:test";
-import { runWithError } from "harness";
+import { bunEnv, bunExe, runWithError } from "harness";
+import dgram from "node:dgram";
 import dns from "node:dns";
+import { once } from "node:events";
 
 describe.each([
   ["dns.Resolver", dns.Resolver],
@@ -46,3 +48,181 @@ describe.each([
     expect(missing).toEqual(runWithError(() => handle[method](undefined)));
   });
 });
+
+/** A UDP socket that never answers: a query sent to it stays pending until `cancel()`. */
+async function listenWithoutAnswering() {
+  const socket = dgram.createSocket("udp4");
+  socket.bind(0, "127.0.0.1");
+  await once(socket, "listening");
+  return { socket, servers: ["127.0.0.1:" + socket.address().port] };
+}
+
+/** Starts a query of `resolver` that settles with the code of its error. */
+const pendingQueries: [string, () => { resolver: any; query: () => Promise<string | undefined> }][] = [
+  [
+    "dns.Resolver",
+    () => {
+      const resolver = new dns.Resolver();
+      const query = () =>
+        new Promise<string | undefined>(resolve =>
+          resolver.resolve4("pending.example.test", err => resolve(err?.code)),
+        );
+      return { resolver, query };
+    },
+  ],
+  [
+    "dns.promises.Resolver",
+    () => {
+      const resolver = new dns.promises.Resolver();
+      const query = () =>
+        resolver.resolve4("pending.example.test").then(
+          () => undefined,
+          err => err.code,
+        );
+      return { resolver, query };
+    },
+  ],
+];
+
+// https://github.com/nodejs/node/pull/65021
+describe("setServers port range", () => {
+  const badPort = {
+    name: "RangeError",
+    code: "ERR_SOCKET_BAD_PORT",
+    message: expect.stringMatching(/^Port should be >= 0 and < 65536\. Received /),
+  };
+
+  describe.each([
+    ["dns.Resolver", dns.Resolver],
+    ["dns.promises.Resolver", dns.promises.Resolver],
+  ])("%s", (_name, Resolver: any) => {
+    // Before the check, 65536 and 2**32 + 53 were the default port, and 99999 was 34463.
+    test.each([
+      ["127.0.0.1:65536", "127.0.0.1:65536"],
+      ["127.0.0.1:99999", "127.0.0.1:99999"],
+      ["127.0.0.1:2**32+53", `127.0.0.1:${2 ** 32 + 53}`],
+      ["127.0.0.1:2**64", "127.0.0.1:18446744073709551616"],
+      ["127.0.0.1:<400 digits>", `127.0.0.1:${Buffer.alloc(400, "9")}`],
+      ["[::1]:65536", "[::1]:65536"],
+      ["[::1]:99999", "[::1]:99999"],
+      ["[::1]:2**32+53", `[::1]:${2 ** 32 + 53}`],
+      ["[::1]:<400 digits>", `[::1]:${Buffer.alloc(400, "9")}`],
+      ["[fe80::1%lo]:99999", "[fe80::1%lo]:99999"],
+    ])("%s throws ERR_SOCKET_BAD_PORT and leaves the servers as they were", (_label, server) => {
+      const resolver = new Resolver();
+      resolver.setServers(["192.0.2.1"]);
+      expect(runWithError(() => resolver.setServers([server]))).toMatchObject(badPort);
+      expect(runWithError(() => resolver.setServers(["192.0.2.2", server]))).toMatchObject(badPort);
+      expect(resolver.getServers()).toEqual(["192.0.2.1"]);
+    });
+
+    test("accepts the ports 0 to 65535", () => {
+      const resolver = new Resolver();
+      resolver.setServers([
+        "127.0.0.1:65535",
+        "[::1]:65535",
+        "127.0.0.2:0",
+        "[::2]:0",
+        "127.0.0.3:00053",
+        "127.0.0.4:000000000000099",
+      ]);
+      expect(resolver.getServers()).toEqual([
+        "127.0.0.1:65535",
+        "[::1]:65535",
+        "127.0.0.2",
+        "::2",
+        "127.0.0.3",
+        "127.0.0.4:99",
+      ]);
+    });
+
+    test("checks the servers in order, and the address of a server before its port", () => {
+      const resolver = new Resolver();
+      resolver.setServers(["192.0.2.1"]);
+      const invalidAddress = (server: string) => ({
+        name: "TypeError",
+        code: "ERR_INVALID_IP_ADDRESS",
+        message: `Invalid IP address: ${server}`,
+      });
+      expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", "nope"]))).toMatchObject(badPort);
+      expect(runWithError(() => resolver.setServers(["1.2.3.4:65536", 5]))).toMatchObject(badPort);
+      expect(runWithError(() => resolver.setServers(["nope", "1.2.3.4:65536"]))).toMatchObject(invalidAddress("nope"));
+      expect(runWithError(() => resolver.setServers(["nope:65536"]))).toMatchObject(invalidAddress("nope:65536"));
+      expect(runWithError(() => resolver.setServers(["[nope]:65536"]))).toMatchObject(invalidAddress("[nope]:65536"));
+      expect(resolver.getServers()).toEqual(["192.0.2.1"]);
+    });
+  });
+
+  test.concurrent.each(pendingQueries)("%s with a pending query reports the port first", async (_name, make) => {
+    const { socket, servers } = await listenWithoutAnswering();
+    const { resolver, query } = make();
+    try {
+      resolver.setServers(servers);
+      const received = once(socket, "message");
+      const settled = query();
+      await received;
+      expect(runWithError(() => resolver.setServers(["1.2.3.4:65536"]))).toMatchObject(badPort);
+      expect(runWithError(() => resolver.setServers(["1.2.3.4"]))).toMatchObject({
+        code: "ERR_DNS_SET_SERVERS_FAILED",
+      });
+      expect(resolver.getServers()).toEqual(servers);
+      resolver.cancel();
+      expect(await settled).toBe("ECANCELLED");
+      expect(runWithError(() => resolver.setServers(["1.2.3.4:65536"]))).toMatchObject(badPort);
+    } finally {
+      resolver.cancel();
+      socket.close();
+    }
+  });
+
+  // These set the default resolver of the whole process. getServers() drops zone ids, so nothing can put it back.
+  test.concurrent("dns.setServers, dns.promises.setServers and a receiver that is no Resolver", async () => {
+    const fixture = `
+      const dns = require("node:dns");
+      const forms = {
+        "dns.setServers": servers => dns.setServers(servers),
+        "dns.promises.setServers": servers => dns.promises.setServers(servers),
+        "Resolver.prototype.setServers.call(undefined)": servers =>
+          dns.Resolver.prototype.setServers.call(undefined, servers),
+      };
+      const errorOf = fn => {
+        try {
+          fn();
+        } catch (error) {
+          return { name: error.name, code: error.code, message: error.message };
+        }
+      };
+      const results = {};
+      for (const [form, setServers] of Object.entries(forms)) {
+        dns.setServers(["192.0.2.1"]);
+        const errors = {
+          ipv4: errorOf(() => setServers(["127.0.0.1:65536"])),
+          ipv6: errorOf(() => setServers(["[::1]:99999"])),
+          afterAnAcceptedServer: errorOf(() => setServers(["192.0.2.2", "127.0.0.1:65536"])),
+          beforeABadAddress: errorOf(() => setServers(["127.0.0.1:65536", "nope"])),
+        };
+        const serversAfterTheErrors = dns.getServers();
+        setServers(["127.0.0.1:65535", "[::1]:0"]);
+        results[form] = { ...errors, serversAfterTheErrors, accepted: dns.getServers() };
+      }
+      console.log(JSON.stringify(results));
+    `;
+    await using proc = Bun.spawn({ cmd: [bunExe(), "-e", fixture], env: bunEnv, stderr: "pipe" });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    const ofEachForm = {
+      ipv4: badPort,
+      ipv6: badPort,
+      afterAnAcceptedServer: badPort,
+      beforeABadAddress: badPort,
+      serversAfterTheErrors: ["192.0.2.1"],
+      accepted: ["127.0.0.1:65535", "::1"],
+    };
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({
+      "dns.setServers": ofEachForm,
+      "dns.promises.setServers": ofEachForm,
+      "Resolver.prototype.setServers.call(undefined)": ofEachForm,
+    });
+    expect(exitCode).toBe(0);
+  });
+});

test/js/node/test/parallel/test-dns.js

@@ -90,6 +90,22 @@ assert(existing.length > 0);
   });
 }
 
+{
+  // Out-of-range ports, which should throw a clean error.
+  const invalidPorts = [2 ** 16, 2 ** 32, 2 ** 64];
+  invalidPorts.forEach((port) => {
+    assert.throws(
+      () => {
+        dns.setServers([`1.2.3.4:${port}`]);
+      },
+      {
+        name: 'RangeError',
+        code: 'ERR_SOCKET_BAD_PORT'
+      }
+    );
+  });
+}
+
 const goog = [
   '8.8.8.8',
   '8.8.4.4',
@@ -136,6 +152,10 @@ const portsExpected = [
 dns.setServers(ports);
 assert.deepStrictEqual(dns.getServers(), portsExpected);
 
+// Port 0 means "use the default port" for c-ares.
+dns.setServers(['4.4.4.4:0', '[2001:4860:4860::8888]:0']);
+assert.deepStrictEqual(dns.getServers(), ['4.4.4.4', '2001:4860:4860::8888']);
+
 dns.setServers([]);
 assert.deepStrictEqual(dns.getServers(), []);