From 763800f0d1346912695622a77754ecc1530e4b40 Mon Sep 17 00:00:00 2001 From: "Jiaxiao (mossaka) Zhou" Date: Tue, 31 Mar 2026 17:24:15 +0000 Subject: [PATCH 1/4] 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 --- src/cli.ts | 7 ++++ src/domain-patterns.test.ts | 64 +++++++++++++++++++++++++++++++++++++ src/domain-patterns.ts | 17 ++++++++++ src/squid-config.test.ts | 38 ++++++++++++++++++++++ src/squid-config.ts | 26 ++++++++++++--- 5 files changed, 147 insertions(+), 5 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 0aa743f8..52ffcdee 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1639,6 +1639,13 @@ program } } + // Reject whitespace and null bytes to prevent Squid config injection + if (/[\s\0]/.test(url)) { + logger.error(`URL pattern contains illegal whitespace or control characters: ${JSON.stringify(url)}`); + logger.error('URL patterns must not contain spaces, tabs, newlines, or null bytes.'); + process.exit(1); + } + // Ensure pattern has a path component (not just domain) const urlWithoutScheme = url.replace(/^https:\/\//, ''); if (!urlWithoutScheme.includes('/')) { diff --git a/src/domain-patterns.test.ts b/src/domain-patterns.test.ts index fc395eec..a15e2ccc 100644 --- a/src/domain-patterns.test.ts +++ b/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(); diff --git a/src/domain-patterns.ts b/src/domain-patterns.ts index 277ddb30..fc0e2a48 100644 --- a/src/domain-patterns.ts +++ b/src/domain-patterns.ts @@ -151,6 +151,23 @@ export function validateDomainOrPattern(input: string): void { throw new Error('Domain cannot be empty'); } + // Reject characters that could inject Squid config directives or tokens. + // Squid config is line-and-space delimited, so whitespace (space, tab, CR, LF), + // null bytes, and comment/quote characters are dangerous. + // This prevents Squid config injection via --allow-domains. + const DANGEROUS_CHARS = /[\s\0"'`;#\\]/; + const match = trimmed.match(DANGEROUS_CHARS); + if (match) { + 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 '${trimmed}': contains invalid character ${charDesc}. ` + + `Domain names must not contain whitespace, quotes, semicolons, backslashes, or control characters.` + ); + } + // Check for overly broad patterns if (trimmed === '*') { throw new Error("Pattern '*' matches all domains and is not allowed"); diff --git a/src/squid-config.test.ts b/src/squid-config.test.ts index 333d73f8..24597a4c 100644 --- a/src/squid-config.test.ts +++ b/src/squid-config.test.ts @@ -1,6 +1,44 @@ 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|whitespace/); + }); + + 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.-]*'; diff --git a/src/squid-config.ts b/src/squid-config.ts index 73deae6a..49e734d4 100644 --- a/src/squid-config.ts +++ b/src/squid-config.ts @@ -53,10 +53,26 @@ interface PatternsByProtocol { both: DomainPattern[]; } +/** + * Defense-in-depth: assert a domain/regex/URL-pattern string is safe for Squid config interpolation. + * Rejects any whitespace (\s covers space, tab, CR, LF, Unicode whitespace) and null bytes. + * Squid config is line-and-space delimited, so any whitespace could inject directives or tokens. + */ +function assertSafeForSquidConfig(value: string): string { + if (/[\s\0]/.test(value)) { + throw new Error( + `SECURITY: Domain or pattern contains whitespace or null bytes 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}`; } @@ -151,7 +167,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`; @@ -262,7 +278,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for allowed domain patterns (HTTP and HTTPS)'); for (const p of patternsByProto.both) { - aclLines.push(`acl allowed_domains_regex dstdom_regex -i ${p.regex}`); + aclLines.push(`acl allowed_domains_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); } } @@ -280,7 +296,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for HTTP-only domain patterns'); for (const p of patternsByProto.http) { - aclLines.push(`acl allowed_http_only_regex dstdom_regex -i ${p.regex}`); + aclLines.push(`acl allowed_http_only_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); } } @@ -298,7 +314,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for HTTPS-only domain patterns'); for (const p of patternsByProto.https) { - aclLines.push(`acl allowed_https_only_regex dstdom_regex -i ${p.regex}`); + aclLines.push(`acl allowed_https_only_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); } } @@ -362,7 +378,7 @@ export function generateSquidConfig(config: SquidConfig): string { blockedAclLines.push(''); blockedAclLines.push('# ACL definitions for blocked domain patterns (wildcard)'); for (const p of blockedPatterns) { - blockedAclLines.push(`acl blocked_domains_regex dstdom_regex -i ${p.regex}`); + blockedAclLines.push(`acl blocked_domains_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); } blockedAccessRules.push('http_access deny blocked_domains_regex'); } From 86db03bbcb10969ee20b518934d9e822df780b0f Mon Sep 17 00:00:00 2001 From: "Jiaxiao (mossaka) Zhou" Date: Tue, 31 Mar 2026 17:50:37 +0000 Subject: [PATCH 2/4] fix: use JSON.stringify in error message to prevent log injection Co-Authored-By: Claude Opus 4.6 --- src/domain-patterns.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/domain-patterns.ts b/src/domain-patterns.ts index fc0e2a48..2523a093 100644 --- a/src/domain-patterns.ts +++ b/src/domain-patterns.ts @@ -158,13 +158,14 @@ export function validateDomainOrPattern(input: string): void { const DANGEROUS_CHARS = /[\s\0"'`;#\\]/; const match = trimmed.match(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 '${trimmed}': contains invalid character ${charDesc}. ` + - `Domain names must not contain whitespace, quotes, semicolons, backslashes, or control characters.` + `Invalid domain ${safeDomainForMessage}: contains invalid character ${charDesc}. ` + + `Domain names must not contain whitespace, quotes, semicolons, backticks, hash characters, backslashes, or control characters.` ); } From 6c22710dcff65afa35109c53041b2f20c7c27817 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 17:13:37 +0000 Subject: [PATCH 3/4] 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 --- src/cli.ts | 10 +++++----- src/domain-patterns.ts | 22 ++++++++++++++++++---- src/squid-config.test.ts | 30 +++++++++++++++++++++++++++++- src/squid-config.ts | 17 +++++++++-------- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 52ffcdee..a8bce586 100644 --- a/src/cli.ts +++ b/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 { OutputFormat } from './types'; import { version } from '../package.json'; @@ -1639,10 +1639,10 @@ program } } - // Reject whitespace and null bytes to prevent Squid config injection - if (/[\s\0]/.test(url)) { - logger.error(`URL pattern contains illegal whitespace or control characters: ${JSON.stringify(url)}`); - logger.error('URL patterns must not contain spaces, tabs, newlines, or null bytes.'); + // 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, backslashes, or null bytes.'); process.exit(1); } diff --git a/src/domain-patterns.ts b/src/domain-patterns.ts index 2523a093..8f07d011 100644 --- a/src/domain-patterns.ts +++ b/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 */ @@ -152,11 +166,11 @@ export function validateDomainOrPattern(input: string): void { } // Reject characters that could inject Squid config directives or tokens. - // Squid config is line-and-space delimited, so whitespace (space, tab, CR, LF), - // null bytes, and comment/quote characters are dangerous. + // 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 DANGEROUS_CHARS = /[\s\0"'`;#\\]/; - const match = trimmed.match(DANGEROUS_CHARS); + 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); diff --git a/src/squid-config.test.ts b/src/squid-config.test.ts index 24597a4c..0a52ac6a 100644 --- a/src/squid-config.test.ts +++ b/src/squid-config.test.ts @@ -26,7 +26,35 @@ describe('defense-in-depth: rejects injected values', () => { sslDbPath: '/tmp/ssl_db', urlPatterns: [maliciousPattern], }); - }).toThrow(/SECURITY|whitespace/); + }).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)', () => { diff --git a/src/squid-config.ts b/src/squid-config.ts index 49e734d4..2c1f193e 100644 --- a/src/squid-config.ts +++ b/src/squid-config.ts @@ -4,6 +4,7 @@ import { isDomainMatchedByPattern, PlainDomainEntry, DomainPattern, + SQUID_DANGEROUS_CHARS, } from './domain-patterns'; import { generateDlpSquidConfig } from './dlp'; @@ -55,13 +56,13 @@ interface PatternsByProtocol { /** * Defense-in-depth: assert a domain/regex/URL-pattern string is safe for Squid config interpolation. - * Rejects any whitespace (\s covers space, tab, CR, LF, Unicode whitespace) and null bytes. - * Squid config is line-and-space delimited, so any whitespace could inject directives or tokens. + * 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 (/[\s\0]/.test(value)) { + if (SQUID_DANGEROUS_CHARS.test(value)) { throw new Error( - `SECURITY: Domain or pattern contains whitespace or null bytes and cannot be ` + + `SECURITY: Domain or pattern contains characters unsafe for Squid config and cannot be ` + `interpolated into squid.conf: ${JSON.stringify(value)}` ); } @@ -278,7 +279,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for allowed domain patterns (HTTP and HTTPS)'); for (const p of patternsByProto.both) { - aclLines.push(`acl allowed_domains_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); + aclLines.push(`acl allowed_domains_regex dstdom_regex -i ${p.regex}`); } } @@ -296,7 +297,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for HTTP-only domain patterns'); for (const p of patternsByProto.http) { - aclLines.push(`acl allowed_http_only_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); + aclLines.push(`acl allowed_http_only_regex dstdom_regex -i ${p.regex}`); } } @@ -314,7 +315,7 @@ export function generateSquidConfig(config: SquidConfig): string { aclLines.push(''); aclLines.push('# ACL definitions for HTTPS-only domain patterns'); for (const p of patternsByProto.https) { - aclLines.push(`acl allowed_https_only_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); + aclLines.push(`acl allowed_https_only_regex dstdom_regex -i ${p.regex}`); } } @@ -378,7 +379,7 @@ export function generateSquidConfig(config: SquidConfig): string { blockedAclLines.push(''); blockedAclLines.push('# ACL definitions for blocked domain patterns (wildcard)'); for (const p of blockedPatterns) { - blockedAclLines.push(`acl blocked_domains_regex dstdom_regex -i ${assertSafeForSquidConfig(p.regex)}`); + blockedAclLines.push(`acl blocked_domains_regex dstdom_regex -i ${p.regex}`); } blockedAccessRules.push('http_access deny blocked_domains_regex'); } From 441af68ae97dc922b2942a3c2e7dd6f9d8ec3a7e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 17:15:15 +0000 Subject: [PATCH 4/4] fix: correct error message for URL pattern validation Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/62b8aef1-1409-4f1d-8641-71dce77c2ceb --- src/cli.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli.ts b/src/cli.ts index a8bce586..bc508e1e 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1642,7 +1642,7 @@ 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, backslashes, or null bytes.'); + logger.error('URL patterns must not contain whitespace, quotes, semicolons, backticks, hash characters, or null bytes.'); process.exit(1); }