From cbda0e2c138723e211f8fcb9374567e08d02efc0 Mon Sep 17 00:00:00 2001 From: Rhett Trappman Date: Mon, 7 Sep 2026 20:22:47 -0600 Subject: [PATCH] fix network monitor command injection --- sensor/NetworkMonitorSensor.js | 19 ++++----- test/test_network_monitor_security.js | 60 +++++++++++++++++++++++++++ util/NetworkMonitorCommand.js | 39 +++++++++++++++++ 3 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 test/test_network_monitor_security.js create mode 100644 util/NetworkMonitorCommand.js diff --git a/sensor/NetworkMonitorSensor.js b/sensor/NetworkMonitorSensor.js index 9278592a48..4ccef37383 100644 --- a/sensor/NetworkMonitorSensor.js +++ b/sensor/NetworkMonitorSensor.js @@ -18,7 +18,7 @@ const log = require('../net2/logger.js')(__filename); const Sensor = require('./Sensor.js').Sensor; -const exec = require('child-process-promise').exec; +const networkMonitorCommand = require('../util/NetworkMonitorCommand.js'); const f = require('../net2/Firewalla.js'); const fc = require('../net2/config.js'); const extensionManager = require('./ExtensionManager.js') @@ -334,11 +334,10 @@ class NetworkMonitorSensor extends Sensor { rtid = intf.rtid; } - const result = await exec(`sudo ping -i ${cfg.sampleTick} ${rtid ? `-m ${rtid}` : ""} -c ${cfg.sampleCount} -W 1 -4 -n ${target}| awk '/time=/ && !/DUP!/ {print $7}' | cut -d= -f2`).catch((err) => { + const data = await networkMonitorCommand.ping(target, cfg.sampleTick, cfg.sampleCount, rtid).catch((err) => { log.error(`ping failed on ${target}:`,err.message); - return null; + return []; } ); - const data = (result && result.stdout) ? result.stdout.trim().split(/\n/).map(e => parseFloat(e)) : []; return { "status": "OK", "data": await this.recordSampleDataInRedis(MONITOR_PING, target, timeSlot, data, cfg, opts)}; } catch (err) { log.error("failed to sample PING:",err.message); @@ -372,13 +371,11 @@ class NetworkMonitorSensor extends Sensor { } } for (let i=0;i { + const queryTime = await networkMonitorCommand.dns(target, bindIP, cfg.sampleTick, cfg.lookupName).catch((err) => { log.error(`dig failed on ${target}:`,err.message); return null; } ); - if (result && result.stdout) { - data.push(parseInt(result.stdout.trim())); - } + if (queryTime !== null) data.push(queryTime); } return { "status": "OK", "data": await this.recordSampleDataInRedis(MONITOR_DNS, target, timeSlot, data, cfg, opts)}; } catch (err) { @@ -454,13 +451,11 @@ class NetworkMonitorSensor extends Sensor { } for (let i=0;i { + const queryTime = await networkMonitorCommand.http(target, bindIP).catch((err) => { log.error(`curl failed on ${target}:`,err.message); return null; } ); - if (result && result.stdout) { - data.push(parseFloat(result.stdout.trim())); - } + if (queryTime !== null && Number.isFinite(queryTime)) data.push(queryTime); } catch (err2) { log.error("curl command failed:",err2); } diff --git a/test/test_network_monitor_security.js b/test/test_network_monitor_security.js new file mode 100644 index 0000000000..1c2145dd50 --- /dev/null +++ b/test/test_network_monitor_security.js @@ -0,0 +1,60 @@ +/* Copyright 2016-2026 Firewalla Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3. + */ + +'use strict'; + +const { expect } = require('chai'); +const proxyquire = require('proxyquire').noPreserveCache(); + +describe('NetworkMonitorSensor command execution', function() { + let calls; + let command; + + beforeEach(function() { + calls = []; + command = proxyquire('../util/NetworkMonitorCommand.js', { + 'child-process-promise': { + execFile: async (file, args) => { + calls.push({ file, args }); + if (file === 'sudo') return { stdout: '64 bytes from host: time=1.25 ms\n' }; + if (file === 'dig') return { stdout: ';; Query time: 7 msec\n' }; + return { stdout: '0.125000\n' }; + } + } + }); + }); + + it('passes ping targets as literal arguments instead of shell commands', async function() { + const target = '1.1.1.1; touch /tmp/network-monitor-injected'; + const result = await command.ping(target, 1, 1, 0); + + expect(calls).to.deep.equal([{ + file: 'sudo', + args: ['ping', '-i', '1', '-c', '1', '-W', '1', '-4', '-n', target] + }]); + expect(result).to.deep.equal([1.25]); + }); + + it('passes DNS names and HTTP URLs as literal arguments', async function() { + const dnsTarget = '8.8.8.8; touch /tmp/network-monitor-injected'; + const lookupName = 'example.com; touch /tmp/network-monitor-injected'; + const httpTarget = "https://example.com/'; touch /tmp/network-monitor-injected; #"; + + const dnsResult = await command.dns(dnsTarget, null, 1, lookupName); + const httpResult = await command.http(httpTarget, null); + + expect(calls[0]).to.deep.equal({ + file: 'dig', + args: [`@${dnsTarget}`, '+tries=1', '+timeout=1', lookupName] + }); + expect(calls[1]).to.deep.equal({ + file: 'curl', + args: ['-s', '-k', '-m', '10', '-o', '/dev/null', '-w', '%{time_total}\n', '--', httpTarget] + }); + expect(dnsResult).to.equal(7); + expect(httpResult).to.equal(0.125); + }); +}); diff --git a/util/NetworkMonitorCommand.js b/util/NetworkMonitorCommand.js new file mode 100644 index 0000000000..c17b38e0fc --- /dev/null +++ b/util/NetworkMonitorCommand.js @@ -0,0 +1,39 @@ +/* Copyright 2016-2026 Firewalla Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3. + */ + +'use strict'; + +const { execFile } = require('child-process-promise'); + +async function ping(target, sampleTick, sampleCount, rtid) { + const args = ['ping', '-i', String(sampleTick)]; + if (rtid) args.push('-m', String(rtid)); + args.push('-c', String(sampleCount), '-W', '1', '-4', '-n', String(target)); + const result = await execFile('sudo', args); + return result.stdout.split(/\n/) + .filter(line => /time=/.test(line) && !/DUP!/.test(line)) + .map(line => Number((line.match(/time[=<]([0-9.]+)/) || [])[1])) + .filter(Number.isFinite); +} + +async function dns(target, bindIP, sampleTick, lookupName) { + const args = [`@${target}`]; + if (bindIP) args.push('-b', String(bindIP)); + args.push('+tries=1', `+timeout=${sampleTick}`, String(lookupName)); + const result = await execFile('dig', args); + const match = result.stdout.match(/Query time:\s+(\d+)\s+msec/); + return match ? Number(match[1]) : null; +} + +async function http(target, bindIP) { + const args = ['-s', '-k', '-m', '10']; + if (bindIP) args.push('--interface', String(bindIP)); + args.push('-o', '/dev/null', '-w', '%{time_total}\n', '--', String(target)); + const result = await execFile('curl', args); + return Number(result.stdout.trim()); +} + +module.exports = { ping, dns, http };