diff --git a/README.md b/README.md index 42dd636..c822bb4 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,8 @@ getLinkPreview("http://maliciousLocalHostRedirection.com", { This might add some latency to your request but prevents loopback attacks. +Note: for `https://` URLs the request is still made against the original hostname (not the resolved IP), since TLS validates the certificate and SNI against the hostname. The resolved address is only used to detect and block loopback/private targets before the request is made. + ## Redirections Same to SSRF, following redirections is dangerous, the library errors by default when the response tries to redirect the user. There are however some simple redirections that are valid (e.g. HTTP to HTTPS) and you might want to allow them, you can do it via: diff --git a/__tests__/index.spec.ts b/__tests__/index.spec.ts index f3232ad..f8c087f 100644 --- a/__tests__/index.spec.ts +++ b/__tests__/index.spec.ts @@ -305,6 +305,36 @@ describe(`#getLinkPreview()`, () => { } }); + it(`should keep the original hostname for HTTPS requests to preserve SNI/TLS`, async () => { + const fetchResponse = new Response( + ` + + + `, + { + headers: { + "content-type": "text/html", + }, + }, + ); + Object.defineProperty(fetchResponse, "url", { + value: "https://example.com/", + }); + const fetchSpy = jest.spyOn(globalThis, "fetch").mockResolvedValue(fetchResponse); + + try { + const response = await getLinkPreview(`https://example.com/`, { + resolveDNSHost: async () => "93.184.216.34", + }); + + expect((response as any).title).toEqual("Resolved host"); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(fetchSpy.mock.calls[0][0]).toEqual("https://example.com/"); + } finally { + fetchSpy.mockRestore(); + } + }); + it(`should block resolved local addresses in normalized IPv6 forms`, async () => { const fetchSpy = jest .spyOn(globalThis, "fetch") diff --git a/index.ts b/index.ts index bcb3bfa..cbccf53 100644 --- a/index.ts +++ b/index.ts @@ -249,6 +249,15 @@ function getValidatedFetchUrl(url: string, resolvedAddressOrUrl?: string) { } const parsedUrl = new URL(url); + + // Rewriting the hostname to a bare IP breaks TLS: the handshake sends the IP as + // the SNI server name and certificate validation fails, since certificates are + // issued for hostnames, not IPs. The resolved address is still used above (via + // throwOnLoopback) to block the request, so HTTPS requests keep their hostname. + if (parsedUrl.protocol === "https:") { + return url; + } + parsedUrl.hostname = formatHostnameForUrl(resolvedAddress); return parsedUrl.href; }