Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,9 @@ after fix:

### .escapeShellArg()

Escape command line arguments. Add single quotes around a string and quotes/escapes any existing single quotes allowing you to pass a string directly to a shell function and having it be treated as a single safe argument.
Escape POSIX shell command line arguments. Add single quotes around a string and quotes/escapes any existing single quotes allowing you to pass a string directly to a shell function and having it be treated as a single safe argument.

Prefer `child_process.execFile()` or `child_process.spawn()` with an arguments array when possible. This helper is for a single argument in a POSIX shell command string, and is not a Windows `cmd.exe` or PowerShell escaping helper.

```js
const ip = '127.0.0.1 && cat /etc/passwd'
Expand Down
4 changes: 3 additions & 1 deletion README.zh-CN.md
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,9 @@ __处理过程较复杂,性能损耗较大,尽量避免使用__

### .escapeShellArg()

命令行参数转义。给字符串增加一对单引号并且能引用或者转码任何已经存在的单引号, 这样以确保能够直接将一个字符串传入 shell 函数,并且还是确保安全的。
POSIX shell 命令行参数转义。给字符串增加一对单引号并且能引用或者转码任何已经存在的单引号, 这样以确保能够直接将一个字符串传入 shell 函数,并且还是确保安全的。

如果可以,优先使用 `child_process.execFile()` 或 `child_process.spawn()` 的参数数组。这个 helper 只用于 POSIX shell 命令字符串里的单个参数,不适用于 Windows `cmd.exe` 或 PowerShell 转义。

```js
const ip = '127.0.0.1 && cat /etc/passwd'
Expand Down
2 changes: 1 addition & 1 deletion lib/helper/escapeShellArg.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
function escapeShellArg(string) {

const str = '' + string;
return '\'' + str.replace(/\\/g, '\\\\').replace(/\'/g, '\\\'') + '\'';
return '\'' + str.replace(/'/g, '\'\\\'\'') + '\'';

}
module.exports = escapeShellArg;
16 changes: 16 additions & 0 deletions test/app/extends/escapeShellArg.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
const assert = require('node:assert/strict');
const { exec: childProcessExec } = require('node:child_process');
const { promisify } = require('node:util');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since this is a backport to the 3.x branch, it may be run on older Node.js versions (such as Node.js 14 < 14.18.0) that do not support the node: prefix or node:assert/strict. To ensure maximum compatibility across all supported Node.js versions in the 3.x release line, it is safer to use standard require statements without the node: prefix.

Suggested change
const assert = require('node:assert/strict');
const { exec: childProcessExec } = require('node:child_process');
const { promisify } = require('node:util');
const assert = require('assert');
const { exec: childProcessExec } = require('child_process');
const { promisify } = require('util');


const mm = require('egg-mock');

const escapeShellArg = require('../../../lib/helper/escapeShellArg');

const exec = promisify(childProcessExec);
const itOnPosix = process.platform === 'win32' ? it.skip : it;

describe('test/app/extends/escapeShellArg.test.js', () => {
let app;
before(() => {
Expand Down Expand Up @@ -33,5 +42,12 @@ describe('test/app/extends/escapeShellArg.test.js', () => {
.expect(200)
.expect('true');
});

itOnPosix('should keep single quotes inside one shell argument', async () => {
const payload = '\'; echo EGG_SECURITY_INJECTED; #';
const { stdout } = await exec(`printf 'ARG:%s\\n' ${escapeShellArg(payload)}`);

assert.equal(stdout, 'ARG:\'; echo EGG_SECURITY_INJECTED; #\n');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use assert.strictEqual instead of assert.equal to ensure strict equality comparison, especially when using the standard assert module instead of the strict assertion mode.

Suggested change
assert.equal(stdout, 'ARG:\'; echo EGG_SECURITY_INJECTED; #\n');
assert.strictEqual(stdout, 'ARG:\'; echo EGG_SECURITY_INJECTED; #\n');

});
});
});
2 changes: 1 addition & 1 deletion test/fixtures/apps/helper-escapeShellArg-app/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = app => {

app.get('/escapeShellArg-2', async function() {
const port = '8889\'|chmod 777 /tmp/muma.sh;echo \\';
this.body = `cp.exec('./start.sh '+${this.helper.escapeShellArg(port)})` === 'cp.exec(\'./start.sh \'+\'8889\\\'|chmod 777 /tmp/muma.sh;echo \\\\\')';
this.body = `cp.exec('./start.sh '+${this.helper.escapeShellArg(port)})` === 'cp.exec(\'./start.sh \'+\'8889\'\\\'\'|chmod 777 /tmp/muma.sh;echo \\\')';
});

app.get('/escapeShellArg-3', async function() {
Expand Down
Loading