Skip to content

Commit

Permalink
sanitizeShellString() and other security improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
sebhildebrandt committed Mar 15, 2021
1 parent b106e41 commit 7922366
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 37 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -77,7 +77,8 @@ For major (breaking) changes - **version 4, 3 and 2** - see end of page.

| Version | Date | Comment |
| -------------- | -------------- | -------- |
| 5.6.3 | 2021-03-10 | `sanitizeShellString()` improvement |
| 5.6.4 | 2021-03-15 | `sanitizeShellString()` and other security improvements |
| 5.6.3 | 2021-03-14 | `sanitizeShellString()` improvement |
| 5.6.2 | 2021-03-10 | `networkInterfaces()` `cpu()` improvement (win) |
| 5.6.1 | 2021-03-03 | `get()` fixed issue boolean parameters |
| 5.6.0 | 2021-03-03 | `cpuTemperature()` added socket and chipset temp (linux) |
Expand Down
5 changes: 5 additions & 0 deletions docs/history.html
Expand Up @@ -56,6 +56,11 @@ <h3>Full version history</h3>
</tr>
</thead>
<tbody>
<tr>
<th scope="row">5.6.4</th>
<td>2021-03-15</td>
<td><span class="code">sanitizeShellString()</span> and other security improvements</td>
</tr>
<tr>
<th scope="row">5.6.3</th>
<td>2021-03-14</td>
Expand Down
4 changes: 2 additions & 2 deletions docs/index.html
Expand Up @@ -166,11 +166,11 @@
<body>
<header class="bg-image-full">
<div class="top-container">
<a href="security.html" class="recommendation">Security advisory:<br>Update to v5.6.3</a>
<a href="security.html" class="recommendation">Security advisory:<br>Update to v5.6.4</a>
<img class="logo" src="assets/logo.png">
<div class="title">systeminformation</div>
<div class="subtitle"><span id="typed"></span>&nbsp;</div>
<div class="version">New Version: <span id="version">5.6.3</span></div>
<div class="version">New Version: <span id="version">5.6.4</span></div>
<button class="btn btn-light" onclick="location.href='https://github.com/sebhildebrandt/systeminformation'">View on Github <i class=" fab fa-github"></i></button>
</div>
<div class="down">
Expand Down
6 changes: 3 additions & 3 deletions docs/security.html
Expand Up @@ -45,16 +45,16 @@
<div class="text">
<h2>Command Injection Vulnerability</h2>
<p><span class="bold">Affected versions:</span>
&lt; 5.6.3 and &lt; 4.34.16<br>
<span class="bold">Date:</span> 2021-03-14<br>
&lt; 5.6.4 and &lt; 4.34.17<br>
<span class="bold">Date:</span> 2021-03-15<br>
<span class="bold">CVE indentifier</span> -
</p>

<h4>Impact</h4>
<p>We had an issue that there was a possibility to perform a potential command injection possibility by passing a manipulated string prototype as a parameter to the following functions. Affected commands: <span class="code">inetLatency()</span>, <span class="code">inetChecksite()</span>, <span class="code">services()</span>, <span class="code">processLoad()</span>.</p>

<h4>Patch</h4>
<p>Problem was fixed with additional parameter checking. Please upgrade to version >= 5.6.3 (or >= 4.34.16 if you are using version 4).</p>
<p>Problem was fixed with additional parameter checking. Please upgrade to version >= 5.6.4 (or >= 4.34.17 if you are using version 4).</p>

<h4>Workarround</h4>
<p>If you cannot upgrade, be sure to check or sanitize parameter strings that are passed to <span class="code">inetLatency()</span>, <span class="code">inetChecksite()</span>, <span class="code">services()</span>, <span class="code">processLoad()</span> (string only)</p>
Expand Down
5 changes: 5 additions & 0 deletions docs/v4/history.html
Expand Up @@ -83,6 +83,11 @@ <h3>Full version history</h3>
</tr>
</thead>
<tbody>
<tr>
<th scope="row">4.34.17</th>
<td>2021-03-14</td>
<td><span class="code">sanitizeShellString()</span> and other security improvements</td>
</tr>
<tr>
<th scope="row">4.34.16</th>
<td>2021-03-14</td>
Expand Down
4 changes: 2 additions & 2 deletions docs/v4/index.html
Expand Up @@ -165,12 +165,12 @@
<body>
<header class="bg-image-full">
<div class="container">
<a href="security.html" class="recommendation">Security advisory:<br>Update to v4.34.16</a>
<a href="security.html" class="recommendation">Security advisory:<br>Update to v4.34.17</a>
<img class="logo" src="assets/logo.png">
<div class="title">systeminformation </div>
<div class="subtitle"><span id="typed"></span>&nbsp;</div>
<div class="version larger">Version 4 documentation</div>
<div class="version">Current Version: <span id="version">4.34.16</span></div>
<div class="version">Current Version: <span id="version">4.34.17</span></div>
<button class="btn btn-light" onclick="location.href='https://github.com/sebhildebrandt/systeminformation'">View on Github <i class=" fab fa-github"></i></button>
</div>
<div class="down">
Expand Down
6 changes: 3 additions & 3 deletions docs/v4/security.html
Expand Up @@ -44,16 +44,16 @@
<div class="text">
<h2>Command Injection Vulnerability</h2>
<p><span class="bold">Affected versions:</span>
&lt; 4.34.13<br>
<span class="bold">Date:</span> 2021-03-14<br>
&lt; 4.34.17<br>
<span class="bold">Date:</span> 2021-03-15<br>
<span class="bold">CVE indentifier</span> -
</p>

<h4>Impact</h4>
<p>We had an issue that there was a possibility to perform a potential command injection possibility by passing a manipulated string prototype as a parameter to the following functions. Affected commands: <span class="code">inetLatency()</span>, <span class="code">inetChecksite()</span>, <span class="code">services()</span>, <span class="code">processLoad()</span>.</p>

<h4>Patch</h4>
<p>Problem was fixed with additional parameter checking. Please upgrade to version >= 4.34.13 if you are using version 4.</p>
<p>Problem was fixed with additional parameter checking. Please upgrade to version >= 4.34.17 if you are using version 4.</p>

<h4>Workarround</h4>
<p>If you cannot upgrade, be sure to check or sanitize parameter strings that are passed to <span class="code">inetLatency()</span>, <span class="code">inetChecksite()</span>, <span class="code">services()</span>, <span class="code">processLoad()</span> (string only)</p>
Expand Down
32 changes: 18 additions & 14 deletions lib/internet.js
Expand Up @@ -13,7 +13,7 @@
// 12. Internet
// ----------------------------------------------------------------------------------

const exec = require('child_process').exec;
// const exec = require('child_process').exec;
const execFile = require('child_process').execFile;
const util = require('./util');

Expand Down Expand Up @@ -46,11 +46,12 @@ function inetChecksite(url, callback) {
}
let urlSanitized = '';
const s = util.sanitizeShellString(url, true);
for (let i = 0; i <= 2000; i++) {
const mathMin = util.mathMin;
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined)) {
s[i].__proto__.toLowerCase = util.stringToLower;
const sl = s[i].toLowerCase();
if (sl && sl[0] && !sl[1]) {
if (sl && sl[0] && !sl[1] && sl[0].length === 1) {
urlSanitized = urlSanitized + sl[0];
}
}
Expand All @@ -65,12 +66,14 @@ function inetChecksite(url, callback) {
}
let t = Date.now();
if (_linux || _freebsd || _openbsd || _netbsd || _darwin || _sunos) {
let args = ' -I --connect-timeout 5 -m 5 ' + urlSanitized + ' 2>/dev/null | head -n 1 | cut -d " " -f2';
let args = ['-I', '--connect-timeout', '5', '-m', '5'];
args.push(urlSanitized);
let cmd = 'curl';
exec(cmd + args, function (error, stdout) {
let statusCode = parseInt(stdout.toString());
util.execSave(cmd, args).then((stdout) => {
const lines = stdout.split('\n');
let statusCode = lines[0] && lines[0].indexOf(' ') >= 0 ? parseInt(lines[0].split(' ')[1], 10) : 404;
result.status = statusCode || 404;
result.ok = !error && (statusCode === 200 || statusCode === 301 || statusCode === 302 || statusCode === 304);
result.ok = (statusCode === 200 || statusCode === 301 || statusCode === 302 || statusCode === 304);
result.ms = (result.ok ? Date.now() - t : null);
if (callback) { callback(result); }
resolve(result);
Expand Down Expand Up @@ -142,7 +145,8 @@ function inetLatency(host, callback) {
}
let hostSanitized = '';
const s = (util.isPrototypePolluted() ? '8.8.8.8' : util.sanitizeShellString(host, true)).trim();
for (let i = 0; i <= 2000; i++) {
const mathMin = util.mathMin;
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined)) {
s[i].__proto__.toLowerCase = util.stringToLower;
const sl = s[i].toLowerCase();
Expand Down Expand Up @@ -171,10 +175,10 @@ function inetLatency(host, callback) {
params = '-c2 -t3 ' + hostSanitized;
filt = 'avg';
}
execFile('ping', params.split(' '), function (error, stdout) {
util.execSave('ping', params.split(' ')).then((stdout) => {
let result = null;
if (!error) {
const lines = stdout.toString().split('\n').filter(line => line.indexOf(filt) >= 0).join('\n');
if (stdout) {
const lines = stdout.split('\n').filter(line => line.indexOf(filt) >= 0).join('\n');

const line = lines.split('=');
if (line.length > 1) {
Expand All @@ -191,10 +195,10 @@ function inetLatency(host, callback) {
if (_sunos) {
const params = '-s -a ' + hostSanitized + ' 56 2';
const filt = 'avg';
execFile('ping', params.split(' '), { timeout: 3000 }, function (error, stdout) {
util.execSave('ping', params.split(' '), { timeout: 3000 }).then((stdout) => {
let result = null;
if (!error) {
const lines = stdout.toString().split('\n').filter(line => line.indexOf(filt) >= 0).join('\n');
if (stdout) {
const lines = stdout.split('\n').filter(line => line.indexOf(filt) >= 0).join('\n');
const line = lines.split('=');
if (line.length > 1) {
const parts = line[1].split('/');
Expand Down
3 changes: 2 additions & 1 deletion lib/network.js
Expand Up @@ -1061,7 +1061,8 @@ function networkStatsSingle(iface) {
process.nextTick(() => {
let ifaceSanitized = '';
const s = util.isPrototypePolluted() ? '---' : util.sanitizeShellString(iface);
for (let i = 0; i <= 2000; i++) {
const mathMin = util.mathMin;
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined)) {
ifaceSanitized = ifaceSanitized + s[i];
}
Expand Down
23 changes: 13 additions & 10 deletions lib/processes.js
Expand Up @@ -111,7 +111,8 @@ function services(srv, callback) {
srvString.__proto__.trim = util.stringTrim;

const s = util.sanitizeShellString(srv);
for (let i = 0; i <= 2000; i++) {
const mathMin = util.mathMin;
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined)) {
srvString = srvString + s[i];
}
Expand Down Expand Up @@ -164,15 +165,15 @@ function services(srv, callback) {
}
}
}
if ((_darwin) && srvString === '*') { // service enumeration mnot yet suported on mac OS
if ((_darwin) && srvString === '*') { // service enumeration not yet suported on mac OS
if (callback) { callback(result); }
resolve(result);
}
let comm = (_darwin) ? 'ps -caxo pcpu,pmem,pid,command' : 'ps -axo pcpu,pmem,pid,command';
let args = (_darwin) ? ['-caxo', 'pcpu,pmem,pid,command'] : ['-axo', 'pcpu,pmem,pid,command'];
if (srvString !== '' && srvs.length > 0) {
exec(comm + ' | grep -v grep | grep -iE "' + srvString + '"', { maxBuffer: 1024 * 20000 }, function (error, stdout) { // lgtm [js/shell-command-constructed-from-input]
if (!error) {
let lines = stdout.toString().replace(/ +/g, ' ').replace(/,+/g, '.').split('\n');
util.execSave('ps', args).then((stdout) => {
if (stdout) {
let lines = stdout.replace(/ +/g, ' ').replace(/,+/g, '.').split('\n');
srvs.forEach(function (srv) {
let ps;
if (_darwin) {
Expand Down Expand Up @@ -267,9 +268,10 @@ function services(srv, callback) {
resolve(result);
}
} else {
exec('ps -o comm | grep -v grep | egrep "' + srvString + '"', { maxBuffer: 1024 * 20000 }, function (error, stdout) { // lgtm [js/shell-command-constructed-from-input]
if (!error) {
let lines = stdout.toString().replace(/ +/g, ' ').replace(/,+/g, '.').split('\n');
args = ['-o', 'comm'];
util.execSave('ps', args).then((stdout) => {
if (stdout) {
let lines = stdout.replace(/ +/g, ' ').replace(/,+/g, '.').split('\n');
srvs.forEach(function (srv) {
let ps = lines.filter(function (e) {
return e.indexOf(srv) !== -1;
Expand Down Expand Up @@ -909,7 +911,8 @@ function processLoad(proc, callback) {
processesString.__proto__.trim = util.stringTrim;

const s = util.sanitizeShellString(proc);
for (let i = 0; i <= 2000; i++) {
const mathMin = util.mathMin;
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined)) {
processesString = processesString + s[i];
}
Expand Down
41 changes: 40 additions & 1 deletion lib/util.js
Expand Up @@ -58,6 +58,7 @@ const stringToString = new String().toString;
const stringSubstr = new String().substr;
const stringTrim = new String().trim;
const stringStartWith = new String().startsWith;
const mathMin = Math.min;

function isFunction(functionToCheck) {
let getType = {};
Expand Down Expand Up @@ -389,6 +390,42 @@ function powerShell(cmd) {
});
}

function execSave(cmd, args, options) {
let result = '';
options = options || {};

return new Promise((resolve) => {
process.nextTick(() => {
try {
const child = spawn(cmd, args, options);

if (child && !child.pid) {
child.on('error', function () {
resolve(result);
});
}
if (child && child.pid) {
child.stdout.on('data', function (data) {
result += data.toString();
});
child.on('close', function () {
child.kill();
resolve(result);
});
child.on('error', function () {
child.kill();
resolve(result);
});
} else {
resolve(result);
}
} catch (e) {
resolve(result);
}
});
});
}

function getCodepage() {
if (_windows) {
if (!codepage) {
Expand Down Expand Up @@ -506,7 +543,7 @@ function countLines(lines, startingWith) {
function sanitizeShellString(str, strict = false) {
const s = str || '';
let result = '';
for (let i = 0; i <= 2000; i++) {
for (let i = 0; i <= mathMin(s.length, 2000); i++) {
if (!(s[i] === undefined ||
s[i] === '>' ||
s[i] === '<' ||
Expand Down Expand Up @@ -925,6 +962,7 @@ exports.wmic = wmic;
exports.darwinXcodeExists = darwinXcodeExists;
exports.getVboxmanage = getVboxmanage;
exports.powerShell = powerShell;
exports.execSave = execSave;
exports.nanoSeconds = nanoSeconds;
exports.countUniqueLines = countUniqueLines;
exports.countLines = countLines;
Expand All @@ -943,5 +981,6 @@ exports.stringToString = stringToString;
exports.stringSubstr = stringSubstr;
exports.stringTrim = stringTrim;
exports.stringStartWith = stringStartWith;
exports.mathMin = mathMin;
exports.WINDIR = WINDIR;
exports.getFilesInPath = getFilesInPath;

0 comments on commit 7922366

Please sign in to comment.