From a0bad6fe060686807610971af1d95aac4c3487f4 Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Wed, 2 Sep 2026 14:30:45 -0600 Subject: [PATCH 1/5] fix: harden MSP domain hostname validation --- cli/src/api/client.js | 53 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/cli/src/api/client.js b/cli/src/api/client.js index e92e544..c30e0e5 100644 --- a/cli/src/api/client.js +++ b/cli/src/api/client.js @@ -2,10 +2,45 @@ const axios = require('axios'); const getCleanDomain = (domain) => { const targetDomain = domain || process.env.FIREWALLA_MSP_ID || 'api.firewalla.net'; - const cleanDomain = targetDomain.replace(/^https?:\/\//, '').replace(/\/$/, ''); + const targetUrl = /^https?:\/\//i.test(targetDomain) + ? targetDomain + : `https://${targetDomain}`; + + let url; + try { + url = new URL(targetUrl); + } catch (_) { + console.error(JSON.stringify({ + error: "Invalid domain", + hint: "For security, only *.firewalla.net domains are allowed" + })); + process.exit(1); + } + + const hostname = url.hostname.toLowerCase(); + + // Security: validate the parsed hostname, not the raw input, to prevent + // URL parser confusion from redirecting the MSP token to an attacker host. + if ( + url.protocol !== 'https:' && + url.protocol !== 'http:' + ) { + console.error(JSON.stringify({ + error: "Invalid domain", + hint: "For security, only *.firewalla.net domains are allowed" + })); + process.exit(1); + } - // Security: Only allow Firewalla domains to prevent token theft - if (!cleanDomain.endsWith('.firewalla.net') && cleanDomain !== 'api.firewalla.net') { + if ( + url.username || + url.password || + url.port || + url.pathname !== '/' || + url.search || + url.hash || + (hostname !== 'api.firewalla.net' && !hostname.endsWith('.firewalla.net')) + ) { console.error(JSON.stringify({ error: "Invalid domain", hint: "For security, only *.firewalla.net domains are allowed" @@ -13,7 +48,7 @@ const getCleanDomain = (domain) => { process.exit(1); } - return cleanDomain; + return hostname; }; const getBaseUrl = (domain) => `https://${getCleanDomain(domain)}/v2`; @@ -21,9 +56,9 @@ const getBaseUrl = (domain) => `https://${getCleanDomain(domain)}/v2`; const getClient = (options = {}) => { const token = process.env.FIREWALLA_MSP_TOKEN; if (!token) { - console.error(JSON.stringify({ - error: "Auth missing.", - hint: "Run: export FIREWALLA_MSP_TOKEN='your_msp_api_token_here' or add to .env" + console.error(JSON.stringify({ + error: "Auth missing.", + hint: "Run: export FIREWALLA_MSP_TOKEN='your_msp_api_token_here' or add to .env" })); process.exit(1); } @@ -51,7 +86,7 @@ const resolveBoxGid = async (input, options) => { const envGid = process.env.FIREWALLA_BOX_GID; if (envGid) return envGid; if (boxes.length === 1) return boxes[0].gid; - + console.error(JSON.stringify({ error: "Ambiguous request. Specify --box ." })); process.exit(1); } @@ -82,4 +117,4 @@ const getClientV1 = (options = {}) => { }); }; -module.exports = { getClient, getClientV1, resolveBoxGid }; \ No newline at end of file +module.exports = { getClient, getClientV1, resolveBoxGid }; From dc9ed17930aa0fa3aced5f1ddccc01e2b380eb23 Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Wed, 2 Sep 2026 14:30:52 -0600 Subject: [PATCH 2/5] test: cover MSP domain hostname validation --- test/client.test.js | 107 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 test/client.test.js diff --git a/test/client.test.js b/test/client.test.js new file mode 100644 index 0000000..3a14d21 --- /dev/null +++ b/test/client.test.js @@ -0,0 +1,107 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); + +const CLIENT_PATH = require.resolve('../cli/src/api/client'); + +function withClient(fn) { + delete require.cache[CLIENT_PATH]; + return fn(require(CLIENT_PATH)); +} + +function assertDomainRejected(input) { + const previousToken = process.env.FIREWALLA_MSP_TOKEN; + const previousExit = process.exit; + const previousError = console.error; + + process.env.FIREWALLA_MSP_TOKEN = 'test-token'; + + let exitCode; + process.exit = (code) => { + exitCode = code; + throw new Error('process.exit'); + }; + console.error = () => {}; + + try { + withClient(({ getClient }) => { + assert.throws( + () => getClient({ domain: input }), + /process\.exit/ + ); + }); + assert.equal(exitCode, 1); + } finally { + process.exit = previousExit; + console.error = previousError; + + if (previousToken === undefined) { + delete process.env.FIREWALLA_MSP_TOKEN; + } else { + process.env.FIREWALLA_MSP_TOKEN = previousToken; + } + } +} + +test('accepts the Firewalla API hostname', () => { + const previousToken = process.env.FIREWALLA_MSP_TOKEN; + process.env.FIREWALLA_MSP_TOKEN = 'test-token'; + + try { + withClient(({ getClient }) => { + const client = getClient({ domain: 'api.firewalla.net' }); + assert.equal(client.defaults.baseURL, 'https://api.firewalla.net/v2'); + }); + } finally { + if (previousToken === undefined) { + delete process.env.FIREWALLA_MSP_TOKEN; + } else { + process.env.FIREWALLA_MSP_TOKEN = previousToken; + } + } +}); + +test('accepts Firewalla subdomains with an optional scheme', () => { + const previousToken = process.env.FIREWALLA_MSP_TOKEN; + process.env.FIREWALLA_MSP_TOKEN = 'test-token'; + + try { + withClient(({ getClient }) => { + const client = getClient({ domain: 'https://msp.example.firewalla.net' }); + assert.equal(client.defaults.baseURL, 'https://msp.example.firewalla.net/v2'); + }); + } finally { + if (previousToken === undefined) { + delete process.env.FIREWALLA_MSP_TOKEN; + } else { + process.env.FIREWALLA_MSP_TOKEN = previousToken; + } + } +}); + +test('rejects URL parser confusion that changes the destination hostname', () => { + const attackerControlledDomains = [ + 'attacker.example?.firewalla.net', + 'attacker.example#.firewalla.net', + 'attacker.example/.firewalla.net', + 'firewalla.net.attacker.example', + 'https://attacker.example?.firewalla.net', + ]; + + for (const input of attackerControlledDomains) { + assertDomainRejected(input); + } +}); + +test('rejects credentials, ports, paths, queries, and fragments', () => { + const invalidDomains = [ + 'user:pass@api.firewalla.net', + 'api.firewalla.net:443', + 'api.firewalla.net/v2', + 'api.firewalla.net?redirect=attacker.example', + 'api.firewalla.net#attacker.example', + ]; + + for (const input of invalidDomains) { + assertDomainRejected(input); + } +}); From 65bbb669ecafc13d51313987f4023cc111c2329b Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Wed, 2 Sep 2026 14:30:57 -0600 Subject: [PATCH 3/5] test: enable Node test runner --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6bbf6ac..2de1b1c 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "fw": "./src/index.js" }, "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" + "test": "node --test" }, "dependencies": { "axios": "^1.6.0", From 6c8e093a50ff092f0ddfd8492d00b2ebb8cf79a1 Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Wed, 2 Sep 2026 14:31:18 -0600 Subject: [PATCH 4/5] test: correct exit assertion From 6ece014c7966472f84de33ba761eba34a8ec0c37 Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Wed, 2 Sep 2026 14:31:35 -0600 Subject: [PATCH 5/5] fix: reject non-hostname MSP domain inputs --- cli/src/api/client.js | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/cli/src/api/client.js b/cli/src/api/client.js index c30e0e5..f335ee9 100644 --- a/cli/src/api/client.js +++ b/cli/src/api/client.js @@ -2,6 +2,21 @@ const axios = require('axios'); const getCleanDomain = (domain) => { const targetDomain = domain || process.env.FIREWALLA_MSP_ID || 'api.firewalla.net'; + const invalidDomain = () => { + console.error(JSON.stringify({ + error: "Invalid domain", + hint: "For security, only *.firewalla.net domains are allowed" + })); + process.exit(1); + }; + + // Accept only a hostname, optionally prefixed by http(s) and/or followed + // by a single trailing slash. Everything else is rejected before URL + // parsing so an explicit default port such as :443 cannot be normalized away. + if (!/^(?:https?:\/\/)?[A-Za-z0-9.-]+\/?$/i.test(targetDomain)) { + invalidDomain(); + } + const targetUrl = /^https?:\/\//i.test(targetDomain) ? targetDomain : `https://${targetDomain}`; @@ -10,28 +25,13 @@ const getCleanDomain = (domain) => { try { url = new URL(targetUrl); } catch (_) { - console.error(JSON.stringify({ - error: "Invalid domain", - hint: "For security, only *.firewalla.net domains are allowed" - })); - process.exit(1); + invalidDomain(); } const hostname = url.hostname.toLowerCase(); // Security: validate the parsed hostname, not the raw input, to prevent // URL parser confusion from redirecting the MSP token to an attacker host. - if ( - url.protocol !== 'https:' && - url.protocol !== 'http:' - ) { - console.error(JSON.stringify({ - error: "Invalid domain", - hint: "For security, only *.firewalla.net domains are allowed" - })); - process.exit(1); - } - if ( url.username || url.password || @@ -41,11 +41,7 @@ const getCleanDomain = (domain) => { url.hash || (hostname !== 'api.firewalla.net' && !hostname.endsWith('.firewalla.net')) ) { - console.error(JSON.stringify({ - error: "Invalid domain", - hint: "For security, only *.firewalla.net domains are allowed" - })); - process.exit(1); + invalidDomain(); } return hostname;