fix: prevent Squid config injection via --allow-domains and --allow-urls (#1517)

* fix: prevent Squid config injection via domain inputs

Malicious input to --allow-domains or --allow-urls containing
whitespace, newlines, or null bytes could inject arbitrary
directives into the generated squid.conf. This adds:

1. Character validation in validateDomainOrPattern() rejects
   whitespace, null bytes, quotes, semicolons, backslashes
2. assertSafeForSquidConfig() in squid-config.ts validates
   every value before interpolation into squid.conf
3. URL pattern validation in cli.ts for --allow-urls

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use JSON.stringify in error message to prevent log injection

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use shared SQUID_DANGEROUS_CHARS for URL/domain validation

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/62b8aef1-1409-4f1d-8641-71dce77c2ceb

* fix: correct error message for URL pattern validation

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/62b8aef1-1409-4f1d-8641-71dce77c2ceb

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

Author: 3 people

src/cli.ts

@@ -22,7 +22,7 @@ import {
 } from './host-iptables';
 import { runMainWorkflow } from './cli-workflow';
 import { redactSecrets } from './redact-secrets';
-import { validateDomainOrPattern } from './domain-patterns';
+import { validateDomainOrPattern, SQUID_DANGEROUS_CHARS } from './domain-patterns';
 import { loadAndMergeDomains } from './rules';
 import { detectHostDnsServers } from './dns-resolver';
 import { OutputFormat } from './types';
@@ -1666,6 +1666,13 @@ program
           }
         }
 
+        // Reject characters that could inject Squid config directives or tokens
+        if (SQUID_DANGEROUS_CHARS.test(url)) {
+          logger.error(`URL pattern contains characters unsafe for Squid config: ${JSON.stringify(url)}`);
+          logger.error('URL patterns must not contain whitespace, quotes, semicolons, backticks, hash characters, or null bytes.');
+          process.exit(1);
+        }
+
         // Ensure pattern has a path component (not just domain)
         const urlWithoutScheme = url.replace(/^https:\/\//, '');
         if (!urlWithoutScheme.includes('/')) {

src/domain-patterns.test.ts

@@ -232,6 +232,70 @@ describe('validateDomainOrPattern', () => {
     });
   });
 
+  describe('rejects injection characters', () => {
+    it('should reject LF in domain', () => {
+      expect(() => validateDomainOrPattern('evil.com\nhttp_access allow all')).toThrow('contains invalid character');
+    });
+
+    it('should reject CR in domain', () => {
+      expect(() => validateDomainOrPattern('evil.com\rhttp_access allow all')).toThrow('contains invalid character');
+    });
+
+    it('should reject CRLF in domain', () => {
+      expect(() => validateDomainOrPattern('evil.com\r\nhttp_access allow all')).toThrow('contains invalid character');
+    });
+
+    it('should reject null bytes', () => {
+      expect(() => validateDomainOrPattern('evil.com\0')).toThrow('contains invalid character');
+    });
+
+    it('should reject tabs', () => {
+      expect(() => validateDomainOrPattern('evil.com\tallowed')).toThrow('contains invalid character');
+    });
+
+    it('should reject interior spaces', () => {
+      expect(() => validateDomainOrPattern('evil.com allowed')).toThrow('contains invalid character');
+    });
+
+    it('should reject space-separated domains (ACL token injection)', () => {
+      expect(() => validateDomainOrPattern('.evil.com .attacker.com')).toThrow('contains invalid character');
+    });
+
+    it('should reject semicolons', () => {
+      expect(() => validateDomainOrPattern('evil.com;rm -rf')).toThrow('contains invalid character');
+    });
+
+    it('should reject hash characters', () => {
+      expect(() => validateDomainOrPattern('evil.com#comment')).toThrow('contains invalid character');
+    });
+
+    it('should reject backslashes', () => {
+      expect(() => validateDomainOrPattern('evil.com\\n')).toThrow('contains invalid character');
+    });
+
+    it('should reject single quotes', () => {
+      expect(() => validateDomainOrPattern("evil.com'")).toThrow('contains invalid character');
+    });
+
+    it('should reject double quotes', () => {
+      expect(() => validateDomainOrPattern('evil.com"')).toThrow('contains invalid character');
+    });
+  });
+
+  describe('accepts valid DNS names with underscores', () => {
+    it('should accept _dmarc.example.com', () => {
+      expect(() => validateDomainOrPattern('_dmarc.example.com')).not.toThrow();
+    });
+
+    it('should accept _acme-challenge.example.com', () => {
+      expect(() => validateDomainOrPattern('_acme-challenge.example.com')).not.toThrow();
+    });
+
+    it('should accept _srv._tcp.example.com', () => {
+      expect(() => validateDomainOrPattern('_srv._tcp.example.com')).not.toThrow();
+    });
+  });
+
   describe('protocol-prefixed domains', () => {
     it('should accept valid http:// prefixed domains', () => {
       expect(() => validateDomainOrPattern('http://github.com')).not.toThrow();

src/domain-patterns.ts

@@ -12,6 +12,20 @@
  *   github.com         -> allow both HTTP and HTTPS (default)
  */
 
+/**
+ * Characters that are dangerous in Squid config files when interpolating domain names
+ * or URL regex patterns. Squid config is line-and-space delimited, so:
+ * - Whitespace (space, tab, CR, LF) can split ACL tokens or inject new directives
+ * - Null bytes may terminate strings unexpectedly
+ * - `#` starts a Squid config comment, truncating the rest of the line
+ * - Quotes (", ', `) and `;` can interfere with config parsing
+ *
+ * Note: backslash is intentionally excluded here because URL regex patterns passed to
+ * `--allow-urls` legitimately use `\` for regex escaping (e.g., `\\.` or `[^\\s]`).
+ * Domain names are additionally validated to reject `\` in validateDomainOrPattern().
+ */
+export const SQUID_DANGEROUS_CHARS = /[\s\0"'`;#]/;
+
 /**
  * Protocol restriction for a domain
  */
@@ -151,6 +165,24 @@ export function validateDomainOrPattern(input: string): void {
     throw new Error('Domain cannot be empty');
   }
 
+  // Reject characters that could inject Squid config directives or tokens.
+  // Also reject backslash: domain names never legitimately contain backslashes,
+  // and they could be used in regex injection if they reach Squid config.
+  // This prevents Squid config injection via --allow-domains.
+  const DOMAIN_DANGEROUS_CHARS = /[\s\0"'`;#\\]/;
+  const match = trimmed.match(DOMAIN_DANGEROUS_CHARS);
+  if (match) {
+    const safeDomainForMessage = JSON.stringify(trimmed);
+    const charCode = match[0].charCodeAt(0);
+    const charDesc = charCode <= 0x20 || charCode === 0x7f
+      ? `U+${charCode.toString(16).padStart(4, '0')}`
+      : `'${match[0]}'`;
+    throw new Error(
+      `Invalid domain ${safeDomainForMessage}: contains invalid character ${charDesc}. ` +
+      `Domain names must not contain whitespace, quotes, semicolons, backticks, hash characters, backslashes, or control characters.`
+    );
+  }
+
   // Check for overly broad patterns
   if (trimmed === '*') {
     throw new Error("Pattern '*' matches all domains and is not allowed");

src/squid-config.test.ts

@@ -1,6 +1,72 @@
 import { generateSquidConfig, generatePolicyManifest } from './squid-config';
 import { SquidConfig } from './types';
 
+describe('defense-in-depth: rejects injected values', () => {
+  const defaultPort = 3128;
+
+  it('should reject newline in domain via validateDomainOrPattern', () => {
+    expect(() => {
+      generateSquidConfig({
+        domains: ['evil.com\nhttp_access allow all'],
+        port: defaultPort,
+      });
+    }).toThrow();
+  });
+
+  it('should reject newline in URL pattern', () => {
+    // URL patterns go through generateSslBumpSection, which interpolates into squid.conf.
+    // The assertSafeForSquidConfig guard should catch this.
+    const maliciousPattern = 'https://evil.com/path\nhttp_access allow all';
+    expect(() => {
+      generateSquidConfig({
+        domains: ['evil.com'],
+        port: defaultPort,
+        sslBump: true,
+        caFiles: { certPath: '/tmp/cert.pem', keyPath: '/tmp/key.pem' },
+        sslDbPath: '/tmp/ssl_db',
+        urlPatterns: [maliciousPattern],
+      });
+    }).toThrow(/SECURITY/);
+  });
+
+  it('should reject hash character in URL pattern (Squid comment injection)', () => {
+    const maliciousPattern = 'https://evil.com/path#http_access allow all';
+    expect(() => {
+      generateSquidConfig({
+        domains: ['evil.com'],
+        port: defaultPort,
+        sslBump: true,
+        caFiles: { certPath: '/tmp/cert.pem', keyPath: '/tmp/key.pem' },
+        sslDbPath: '/tmp/ssl_db',
+        urlPatterns: [maliciousPattern],
+      });
+    }).toThrow(/SECURITY/);
+  });
+
+  it('should reject semicolon in URL pattern (Squid token injection)', () => {
+    const maliciousPattern = 'https://evil.com/path;injected';
+    expect(() => {
+      generateSquidConfig({
+        domains: ['evil.com'],
+        port: defaultPort,
+        sslBump: true,
+        caFiles: { certPath: '/tmp/cert.pem', keyPath: '/tmp/key.pem' },
+        sslDbPath: '/tmp/ssl_db',
+        urlPatterns: [maliciousPattern],
+      });
+    }).toThrow(/SECURITY/);
+  });
+
+  it('should reject space in domain (ACL token injection)', () => {
+    expect(() => {
+      generateSquidConfig({
+        domains: ['.evil.com .attacker.com'],
+        port: defaultPort,
+      });
+    }).toThrow();
+  });
+});
+
 // Pattern constant for the safer domain character class (matches the implementation)
 const DOMAIN_CHAR_PATTERN = '[a-zA-Z0-9.-]*';
 

src/squid-config.ts

@@ -4,6 +4,7 @@ import {
   isDomainMatchedByPattern,
   PlainDomainEntry,
   DomainPattern,
+  SQUID_DANGEROUS_CHARS,
 } from './domain-patterns';
 import { generateDlpSquidConfig } from './dlp';
 import { DEFAULT_DNS_SERVERS } from './dns-resolver';
@@ -54,10 +55,26 @@ interface PatternsByProtocol {
   both: DomainPattern[];
 }
 
+/**
+ * Defense-in-depth: assert a domain/regex/URL-pattern string is safe for Squid config interpolation.
+ * Rejects whitespace, null bytes, quotes, semicolons, backticks, hash characters, and backslashes —
+ * all of which can inject directives, tokens, or comments into Squid config.
+ */
+function assertSafeForSquidConfig(value: string): string {
+  if (SQUID_DANGEROUS_CHARS.test(value)) {
+    throw new Error(
+      `SECURITY: Domain or pattern contains characters unsafe for Squid config and cannot be ` +
+      `interpolated into squid.conf: ${JSON.stringify(value)}`
+    );
+  }
+  return value;
+}
+
 /**
  * Helper to add leading dot to domain for Squid subdomain matching
  */
 function formatDomainForSquid(domain: string): string {
+  assertSafeForSquidConfig(domain);
   return domain.startsWith('.') ? domain : `.${domain}`;
 }
 
@@ -152,7 +169,7 @@ function generateSslBumpSection(
   let urlAccessRules = '';
   if (urlPatterns && urlPatterns.length > 0) {
     const urlAcls = urlPatterns
-      .map((pattern, i) => `acl allowed_url_${i} url_regex ${pattern}`)
+      .map((pattern, i) => `acl allowed_url_${i} url_regex ${assertSafeForSquidConfig(pattern)}`)
       .join('\n');
     urlAclSection = `\n# URL pattern ACLs for HTTPS content inspection\n${urlAcls}\n`;