Skip to content
Permalink
Browse files Browse the repository at this point in the history
Strip null characters from arguments
Update both the Unix and Windows `escapeShellArg` implementation to
strip null characters from input. Null characters can be used to at
least cause errors and potentially execute arbitrary commands (depending
on the shell).

This bug was found due to the new fuzzing logic.
  • Loading branch information
ericcornelissen committed Mar 13, 2021
1 parent 57075c4 commit 07a069a
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -7,7 +7,7 @@ Versioning].

## [Unreleased]

- _No changes yet_
- Strip null characters from arguments.

## [1.1.2] - 2021-01-07

Expand Down
2 changes: 1 addition & 1 deletion src/unix.js
Expand Up @@ -11,7 +11,7 @@
* @returns {string} The escaped argument.
*/
function escapeShellArg(arg) {
return arg.replace(/'/g, `'\\''`);
return arg.replace(/\u{0}/gu, "").replace(/'/g, `'\\''`);
}

module.exports.escapeShellArg = escapeShellArg;
2 changes: 1 addition & 1 deletion src/win.js
Expand Up @@ -11,7 +11,7 @@
* @returns {string} The escaped argument.
*/
function escapeShellArg(arg) {
return arg.replace(/"/g, `""`);
return arg.replace(/\u{0}/gu, "").replace(/"/g, `""`);
}

module.exports.escapeShellArg = escapeShellArg;
16 changes: 16 additions & 0 deletions test/unix.test.js
Expand Up @@ -22,4 +22,20 @@ describe("unix.js", function () {
assert.strictEqual(output, `'\\'' & echo '\\''Hello world!'\\''`);
});
});

describe("null characters", function () {
const nullChar = String.fromCharCode(0);

it("removes one null character", function () {
const input = `foo' && ls${nullChar} -al ; echo 'bar`;
const output = escapeShellArg(input);
assert.strictEqual(output, `foo'\\'' && ls -al ; echo '\\''bar`);
});

it("removes multiple null character", function () {
const input = `foo'${nullChar}&&ls -al${nullChar};echo 'bar`;
const output = escapeShellArg(input);
assert.strictEqual(output, `foo'\\''&&ls -al;echo '\\''bar`);
});
});
});
16 changes: 16 additions & 0 deletions test/win.test.js
Expand Up @@ -22,4 +22,20 @@ describe("win.js", function () {
assert.strictEqual(output, `"" & echo ""Hello world!`);
});
});

describe("null characters", function () {
const nullChar = String.fromCharCode(0);

it("removes one null character", function () {
const input = `foo" && ls${nullChar} -al ; echo "bar`;
const output = escapeShellArg(input);
assert.strictEqual(output, `foo"" && ls -al ; echo ""bar`);
});

it("removes multiple null character", function () {
const input = `foo"${nullChar}&&ls -al${nullChar};echo "bar`;
const output = escapeShellArg(input);
assert.strictEqual(output, `foo""&&ls -al;echo ""bar`);
});
});
});

0 comments on commit 07a069a

Please sign in to comment.