Skip to content

Commit 7a4fa93

Browse files
Improve fuzz targets (#318)
* Stop not dealing with newlines for most fuzzing Update most fuzz targets so that they now try out arguments containing newlines. The exceptions to this are the fuzz targets with interpolation set to true, these still ignore newlines (this is because in this scenario newlines are not handled well yet, further work required); and whenever fuzzing with cmd.exe (this is because cmd.exe does not handle newlines well even when arguments are quoted, further work required) (a item, "ec246439...", was added to the fuzz corpus for this). * Align exec fuzz target with exec/execSync recipes Expand the fuzz target for child_process.exec to include a check for usage without having a shell specified (both with and without the interpolation option set). Also, be more methodical about the checks, following the "design" of the other fuzz targets. * Fix incorrect argument preparation and expected output Update the `prepareArg` and `getExpectedOutput` to take more inputs to better understand how to respond. In particular, the inclusion of the `shell` value ensures the correct return value is provided when a shell **is** configured for fuzzing, but the particular fuzz check intents to test without specifying a shell. * Stop not dealing with (most) whitespace for most fuzzing interpolation Update the fuzz checks that set interpolation to true to mostly start fuzzing with whitespace, except for newline (see first paragraph). This required some changes to the implementation (fixed in other branches merged prior to this one). Multiple fuzz targets where added as a result of this, namely: - Related to whitespace between arguments - "377f0b03272b8e006a7ba3fb993328a87e7914075caae18209c7c8c1be205e14" - "38af29e913078ad07fc8d4156c2dfa3942e159bfcebe18e7e562ea3d82fce781" - "cb41f974ee87bf382bc82a4e275bea2ffb99d9bb8d502826f5b6d231250e20b7" - Related to (shell-dependent) special characters after whitespace - "74cd192a0b85f46497a85e0b229630c55c306ce8904ca3d7e362c0ad81fa1088" - "ec246439c059f17f708aa39f76f88ec8d8eaee7ccca2f07d4c09ef57ae6503d8" - Related to trimming whitespace at the start and end of the argumens - "bb9310963784ca202fd80f96f12a3cd13ee11f96a226d7cff5a921902a6b6324" - "f5520c691959b1105525b46c01e6055916ea029f439a5ccefd17b03742adef64"
1 parent 994af92 commit 7a4fa93

12 files changed

+141
-62
lines changed

test/fuzz/_common.cjs

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -11,75 +11,102 @@ require("dotenv").config();
1111
const constants = require("../_constants.cjs");
1212

1313
const ECHO_SCRIPT = constants.echoScript;
14-
const WHITESPACE_REGEX = /\s|\u0085/gu;
15-
16-
function getExpectedOutput(arg) {
17-
return (
18-
arg
19-
.replace(/[\n\r]+/g, "") // Avoid dealing with newlines
20-
.replace(/\u{0}/gu, "") + // Remove null characters
21-
"\n" // Append a newline, like the echo script
22-
);
14+
15+
function isWindows() {
16+
return os.platform() === "win32";
17+
}
18+
19+
function isShellCmd(shell) {
20+
return (isWindows() && shell === undefined) || /cmd\.exe$/.test(shell);
21+
}
22+
23+
function isShellPowerShell(shell) {
24+
return /powershell\.exe$/.test(shell);
25+
}
26+
27+
function getExpectedOutput({ arg, shell }, normalizeWhitespace) {
28+
if (isShellCmd(shell)) {
29+
arg = arg.replace(/[\n\r]+/g, ""); // Remove newline characters, like prep
30+
}
31+
32+
arg = arg.replace(/\u{0}/gu, ""); // Remove null characters, like Shescape
33+
34+
if (normalizeWhitespace) {
35+
// The characters to normalize depend on the shell
36+
// Trim the string like any shell would
37+
if (isShellPowerShell(shell)) {
38+
arg = arg.replace(/^[\s\u0085]+|[\s\u0085]+$/g, "");
39+
} else {
40+
arg = arg.replace(/^[ \t]+|[ \t]+$/g, "");
41+
}
42+
43+
// Convert spacing between arguments to a single space, like the shell would
44+
if (isShellPowerShell(shell)) {
45+
arg = arg.replace(/(\s|\u0085)+/g, " ");
46+
} else {
47+
arg = arg.replace(/[ \t]+/g, " ");
48+
}
49+
}
50+
51+
arg = `${arg}\n`; // Append a newline, like the echo script
52+
return arg;
2353
}
2454

2555
function getFuzzShell() {
2656
return process.env.FUZZ_SHELL;
2757
}
2858

29-
function prepareArg(arg, quoted, disableExtraWindowsPreparations) {
30-
WHITESPACE_REGEX.lastIndex = 0;
31-
32-
const shell = getFuzzShell();
33-
const isWindows = () => os.platform() === "win32";
34-
const isShellCmd = () => shell === undefined || /cmd\.exe$/.test(shell);
35-
const isShellPowerShell = () => /powershell\.exe$/.test(shell);
59+
function prepareArg({ arg, quoted, shell }, disableExtraWindowsPreparations) {
60+
if (isShellCmd(shell)) {
61+
// In CMD ignores everything after a newline (\n) character. This alteration
62+
// is required even when `disableExtraWindowsPreparations` is true.
63+
arg = arg.replace(/[\n\r]+/g, "");
64+
}
3665

37-
let result = arg.replace(/[\n\r]+/g, ""); // Avoid dealing with newlines
3866
if (isWindows() && !disableExtraWindowsPreparations) {
3967
// Node on Windows ...
40-
if (isShellCmd()) {
68+
if (isShellCmd(shell)) {
4169
// ... in CMD, depending on if the argument is quotes ...
4270
if (quoted) {
4371
// ... interprets arguments with `\"` as `"` (even if there's a
4472
// null character between `\` and `"`) so we escape the `\`.
45-
result = result.replace(/((\\\u{0}*)+)(?=\u{0}*("|$))/gu, "$1$1");
73+
arg = arg.replace(/((\\\u{0}*)+)(?=\u{0}*("|$))/gu, "$1$1");
4674
} else {
4775
// ... interprets arguments with `\"` as `"` so we escape the `\` ...
48-
result = result.replace(/((\\\u{0}*)+)(?=\u{0}*")/gu, "$1$1");
76+
arg = arg.replace(/((\\\u{0}*)+)(?=\u{0}*")/gu, "$1$1");
4977

5078
// ... interprets arguments with `"` as `` so we escape it with `\`.
51-
result = result.replace(/"/g, `\\"`);
79+
arg = arg.replace(/"/g, `\\"`);
5280
}
53-
} else if (isShellPowerShell()) {
81+
} else if (isShellPowerShell(shell)) {
5482
// ... in PowerShell, depending on if there's whitespace in the
5583
// argument ...
56-
if (WHITESPACE_REGEX.test(result)) {
84+
if (/\s|\u0085/g.test(arg) && quoted) {
5785
// ... interprets arguments with `""` as nothing so we escape it with
5886
// extra double quotes as `""""` ...
59-
result = result.replace(/"/g, `""`);
87+
arg = arg.replace(/"/g, `""`);
6088

6189
// ... and interprets arguments with `\"` as `"` (even if there's a null
6290
// character between `\` and `"`) so we escape the `\`.
63-
result = result.replace(/((\\\u{0}*)+)(?=\u{0}*("|$))/gu, "$1$1");
91+
arg = arg.replace(/((\\\u{0}*)+)(?=\u{0}*("|$))/gu, "$1$1");
6492
} else {
6593
// ... interprets arguments with `\"` as `"` (even if there's a null
6694
// character between `\` and `"`) so we escape the `\`, except that the
6795
// quote closing the argument cannot be escaped ...
68-
result = result.replace(/((\\\u{0}*)+)(?=\u{0}*("))/gu, "$1$1");
96+
arg = arg.replace(/((\\\u{0}*)+)(?=\u{0}*("))/gu, "$1$1");
6997

7098
// ... and interprets arguments with `""` as nothing so we escape it
7199
// with `\"`.
72-
result = result.replace(/"/g, `\\"`);
100+
arg = arg.replace(/"/g, `\\"`);
73101
}
74102
}
75103
}
76104

77-
return result;
105+
return arg;
78106
}
79107

80108
module.exports = {
81109
ECHO_SCRIPT,
82-
WHITESPACE_REGEX,
83110
getExpectedOutput,
84111
getFuzzShell,
85112
prepareArg,
Binary file not shown.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
��n'Y. �0�
Binary file not shown.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#��(�"�1Ww�
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
��n'Y. �0�

test/fuzz/corpus/ec246439c059f17f708aa39f76f88ec8d8eaee7ccca2f07d4c09ef57ae6503d8

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
#����"3Ww��
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
��K����l����

test/fuzz/exec.test.cjs

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,43 +11,84 @@ const common = require("./_common.cjs");
1111

1212
const shescape = require("../../index.cjs");
1313

14-
function checkEscapesCorrectly(arg, options) {
15-
arg = arg.replace(common.WHITESPACE_REGEX, "");
14+
function checkWithoutShell(arg) {
15+
const argInfo = { arg, shell: undefined, quoted: true };
1616

17-
const preparedArg = common.prepareArg(arg, false);
17+
const preparedArg = common.prepareArg(argInfo);
18+
const quotedArg = shescape.quote(preparedArg);
19+
20+
const stdout = execSync(`node ${common.ECHO_SCRIPT} ${quotedArg}`);
21+
22+
const result = stdout.toString();
23+
const expected = common.getExpectedOutput(argInfo);
24+
assert.strictEqual(result, expected);
25+
}
26+
27+
function checkWithShell(arg) {
28+
const shell = common.getFuzzShell() || true;
29+
const argInfo = { arg, shell, quoted: true };
30+
const execOptions = { shell };
31+
32+
const preparedArg = common.prepareArg(argInfo);
33+
const quotedArg = shescape.quote(preparedArg, execOptions);
34+
35+
const stdout = execSync(
36+
`node ${common.ECHO_SCRIPT} ${quotedArg}`,
37+
execOptions
38+
);
39+
40+
const result = stdout.toString();
41+
const expected = common.getExpectedOutput(argInfo);
42+
assert.strictEqual(result, expected);
43+
}
44+
45+
function checkWithoutShellUsingInterpolation(arg) {
46+
arg = arg.replace(/[\n\r]+/g, "");
47+
48+
const argInfo = { arg, shell: undefined, quoted: false };
49+
50+
const preparedArg = common.prepareArg(argInfo);
1851
const escapedArg = shescape.escape(preparedArg, {
19-
...options,
2052
interpolation: true,
2153
});
2254

23-
const stdout = execSync(`node ${common.ECHO_SCRIPT} ${escapedArg}`, options);
55+
const stdout = execSync(`node ${common.ECHO_SCRIPT} ${escapedArg}`);
2456

2557
const result = stdout.toString();
26-
const expected = common.getExpectedOutput(arg);
58+
const expected = common.getExpectedOutput(argInfo, true);
2759
assert.strictEqual(result, expected);
2860
}
2961

30-
function checkQuotesAndEscapesCorrectly(arg, options) {
31-
const preparedArg = common.prepareArg(arg, true);
32-
const quotedArg = shescape.quote(preparedArg, {
33-
...options,
62+
function checkWithShellUsingInterpolation(arg) {
63+
arg = arg.replace(/[\n\r]+/g, "");
64+
65+
const shell = common.getFuzzShell() || true;
66+
const argInfo = { arg, shell, quoted: false };
67+
const execOptions = { shell };
68+
69+
const preparedArg = common.prepareArg(argInfo);
70+
const escapedArg = shescape.escape(preparedArg, {
71+
...execOptions,
72+
interpolation: true,
3473
});
3574

36-
const stdout = execSync(`node ${common.ECHO_SCRIPT} ${quotedArg}`, options);
75+
const stdout = execSync(
76+
`node ${common.ECHO_SCRIPT} ${escapedArg}`,
77+
execOptions
78+
);
3779

3880
const result = stdout.toString();
39-
const expected = common.getExpectedOutput(arg);
81+
const expected = common.getExpectedOutput(argInfo, true);
4082
assert.strictEqual(result, expected);
4183
}
4284

4385
function fuzz(buf) {
4486
const arg = buf.toString();
45-
const options = {
46-
shell: common.getFuzzShell(),
47-
};
4887

49-
checkEscapesCorrectly(arg, options);
50-
checkQuotesAndEscapesCorrectly(arg, options);
88+
checkWithoutShell(arg);
89+
checkWithShell(arg);
90+
checkWithoutShellUsingInterpolation(arg);
91+
checkWithShellUsingInterpolation(arg);
5192
}
5293

5394
module.exports = {

test/fuzz/execFile.test.cjs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,26 @@ const common = require("./_common.cjs");
1212
const shescape = require("../../index.cjs");
1313

1414
function checkWithoutShell(arg) {
15-
const preparedArg = common.prepareArg(arg, false, true);
15+
const argInfo = { arg, shell: undefined, quoted: false };
16+
17+
const preparedArg = common.prepareArg(argInfo, true);
1618

1719
const stdout = execFileSync(
1820
"node",
1921
shescape.escapeAll([common.ECHO_SCRIPT, preparedArg])
2022
);
2123

2224
const result = stdout.toString();
23-
const expected = common.getExpectedOutput(arg);
25+
const expected = common.getExpectedOutput(argInfo);
2426
assert.strictEqual(result, expected);
2527
}
2628

2729
function checkWithShell(arg) {
28-
const spawnOptions = {
29-
shell: common.getFuzzShell() || true,
30-
};
30+
const shell = common.getFuzzShell() || true;
31+
const argInfo = { arg, shell, quoted: true };
32+
const spawnOptions = { shell };
3133

32-
const preparedArg = common.prepareArg(arg, true, true);
34+
const preparedArg = common.prepareArg(argInfo, true);
3335

3436
const stdout = execFileSync(
3537
"node",
@@ -38,7 +40,7 @@ function checkWithShell(arg) {
3840
);
3941

4042
const result = stdout.toString();
41-
const expected = common.getExpectedOutput(arg);
43+
const expected = common.getExpectedOutput(argInfo);
4244
assert.strictEqual(result, expected);
4345
}
4446

0 commit comments

Comments
 (0)