Skip to content

Commit eeef37e

Browse files
shivammathurshivammathur
committed
GHSA-pqwm-q9pv-ph8r - Fix CWE-78 [skip ci]
1 parent 0dc3306 commit eeef37e

8 files changed

Lines changed: 199 additions & 67 deletions

File tree

‎__tests__/config.test.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,18 @@ import * as config from '../src/config';
22

33
describe('Config tests', () => {
44
it.each`
5-
ini_values | os | output
6-
${'a=b, c=d'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=b\nc=d"'}
7-
${'a=b, c=d'} | ${'linux'} | ${'echo "a=b\nc=d" | sudo tee -a "${pecl_file:-${ini_file[@]}}"'}
8-
${'a=b, c=d'} | ${'darwin'} | ${'echo "a=b\nc=d" | sudo tee -a "${pecl_file:-${ini_file[@]}}"'}
9-
${'a=b & ~c'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'b & ~c\'"'}
10-
${'a="~(b)"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'~(b)\'"'}
11-
${'a="b, c"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=b, c"'}
12-
${'a=b, c=d'} | ${'openbsd'} | ${'Platform openbsd is not supported'}
5+
ini_values | os | output
6+
${'a=b, c=d'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=b\nc=d"'}
7+
${'a=b, c=d'} | ${'linux'} | ${'echo "a=b\nc=d" | sudo tee -a "${pecl_file:-${ini_file[@]}}"'}
8+
${'a=b, c=d'} | ${'darwin'} | ${'echo "a=b\nc=d" | sudo tee -a "${pecl_file:-${ini_file[@]}}"'}
9+
${'a=b & ~c'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'b & ~c\'"'}
10+
${'a="~(b)"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'~(b)\'"'}
11+
${'a="b, c"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=b, c"'}
12+
${'disable_functions="exec,system"'} | ${'linux'} | ${'echo "disable_functions=exec,system" | sudo tee -a'}
13+
${'disable_functions="exec,system"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "disable_functions=exec,system"'}
14+
${'a=$(id)'} | ${'linux'} | ${'echo "a=\'\\$(id)\'"'}
15+
${'a=$(id)'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'`$(id)\'"'}
16+
${'a=b, c=d'} | ${'openbsd'} | ${'Platform openbsd is not supported'}
1317
`('checking addINIValues on $os', async ({ini_values, os, output}) => {
1418
expect(await config.addINIValues(ini_values, os)).toContain(output);
1519
});

‎__tests__/tools.test.ts‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ describe('Tools tests', () => {
187187
${'1.2.3-dev'} | ${'tool'} | ${'phar'} | ${'1.2.3-dev'}
188188
${'1.2.3-alpha1'} | ${'tool'} | ${'phar'} | ${'1.2.3-alpha1'}
189189
${'1.2.3-alpha.1'} | ${'tool'} | ${'phar'} | ${'1.2.3-alpha.1'}
190+
${'1.>=0'} | ${'tool'} | ${'phar'} | ${'1.0'}
190191
`(
191192
'checking getVersion: $version, $tool, $type',
192193
async ({version, tool, type, expected}) => {
@@ -304,22 +305,30 @@ describe('Tools tests', () => {
304305
});
305306

306307
it.each`
307-
os | script | scope
308-
${'linux'} | ${'add_composer_tool tool tool:1.2.3 user/ global'} | ${'global'}
309-
${'darwin'} | ${'add_composer_tool tool tool:1.2.3 user/ scoped'} | ${'scoped'}
310-
${'win32'} | ${'Add-ComposerTool tool tool:1.2.3 user/ scoped'} | ${'scoped'}
311-
${'openbsd'} | ${'Platform openbsd is not supported'} | ${'global'}
312-
`('checking addPackage: $os, $scope', async ({os, script, scope}) => {
313-
const data = getData({
314-
tool: 'tool',
315-
version: '1.2.3',
316-
repository: 'user/tool',
317-
os: os,
318-
scope: scope
319-
});
320-
data['release'] = [data['tool'], data['version']].join(':');
321-
expect(await tools.addPackage(data)).toContain(script);
322-
});
308+
os | release | scope | script
309+
${'linux'} | ${'tool:1.2.3'} | ${'global'} | ${'add_composer_tool tool tool:1.2.3 user/ global'}
310+
${'darwin'} | ${'tool:1.2.3'} | ${'scoped'} | ${'add_composer_tool tool tool:1.2.3 user/ scoped'}
311+
${'win32'} | ${'tool:1.2.3'} | ${'scoped'} | ${'Add-ComposerTool tool tool:1.2.3 user/ scoped'}
312+
${'linux'} | ${'tool:>=1.2'} | ${'global'} | ${'add_composer_tool tool "tool:>=1.2" user/ global'}
313+
${'win32'} | ${'tool:>=1.2'} | ${'global'} | ${'Add-ComposerTool tool "tool:>=1.2" user/ global'}
314+
${'linux'} | ${'tool:1.*'} | ${'global'} | ${'add_composer_tool tool "tool:1.*" user/ global'}
315+
${'linux'} | ${'psalm:^5||^6'} | ${'global'} | ${'add_composer_tool tool "psalm:^5||^6" user/ global'}
316+
${'linux'} | ${'psalm:>=5,<6'} | ${'global'} | ${'add_composer_tool tool "psalm:>=5,<6" user/ global'}
317+
${'openbsd'} | ${'tool:1.2.3'} | ${'global'} | ${'Platform openbsd is not supported'}
318+
`(
319+
'checking addPackage: $os, $release',
320+
async ({os, release, scope, script}) => {
321+
const data = getData({
322+
tool: 'tool',
323+
version: '1.2.3',
324+
repository: 'user/tool',
325+
os,
326+
scope
327+
});
328+
data['release'] = release;
329+
expect(await tools.addPackage(data)).toContain(script);
330+
}
331+
);
323332

324333
it.each`
325334
version | php_version | os | script
@@ -651,7 +660,7 @@ describe('Tools tests', () => {
651660
'add_devtools phpize',
652661
'add_tool https://github.com/phpmd/phpmd/releases/latest/download/phpmd.phar phpmd "--version"',
653662
'add_tool https://github.com/phpspec/phpspec/releases/latest/download/phpspec.phar phpspec "-V"',
654-
'add_composer_tool phpunit-bridge phpunit-bridge:5.6.* symfony/ global',
663+
'add_composer_tool phpunit-bridge "phpunit-bridge:5.6.*" symfony/ global',
655664
'add_composer_tool phpunit-polyfills phpunit-polyfills:1.0.1 yoast/ global',
656665
'add_protoc 1.2.3',
657666
'add_tool https://github.com/vimeo/psalm/releases/latest/download/psalm.phar psalm "-v"',
@@ -711,7 +720,7 @@ describe('Tools tests', () => {
711720
'Add-ComposerTool codeception codeception codeception/ global',
712721
'Add-ComposerTool prestissimo prestissimo hirak/ global',
713722
'Add-ComposerTool automatic-composer-prefetcher automatic-composer-prefetcher narrowspark/ global',
714-
'Add-ComposerTool phinx phinx:1.2.* robmorgan/ scoped',
723+
'Add-ComposerTool phinx "phinx:1.2.*" robmorgan/ scoped',
715724
'Add-ComposerTool phinx phinx:^1.2 robmorgan/ global',
716725
'Add-ComposerTool tool tool:1.2.3 user/ global',
717726
'Add-ComposerTool tool tool:~1.2 user/ global'

‎__tests__/utils.test.ts‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,19 @@ describe('Utils tests', () => {
4040
expect(await utils.parseVersion('7')).toBe('7.0');
4141
expect(await utils.parseVersion('7.4')).toBe('7.4');
4242
expect(await utils.parseVersion('5.x')).toBe('5.6');
43-
expect(await utils.parseVersion('4.x')).toBe(undefined);
43+
expect(await utils.parseVersion('pre')).toBe('pre');
44+
expect(await utils.parseVersion('pre-installed')).toBe('pre');
45+
await expect(utils.parseVersion('4.x')).rejects.toThrow(
46+
'Invalid PHP version: 4.x'
47+
);
48+
await expect(utils.parseVersion('foo')).rejects.toThrow(
49+
'Invalid PHP version:'
50+
);
51+
52+
fetchSpy.mockResolvedValue({data: '{ "latest": "8.1.0" }'});
53+
await expect(utils.parseVersion('latest')).rejects.toThrow(
54+
'Invalid PHP version in manifest:'
55+
);
4456

4557
fetchSpy.mockReset();
4658
fetchSpy.mockResolvedValueOnce({}).mockResolvedValueOnce({});
@@ -56,6 +68,12 @@ describe('Utils tests', () => {
5668
expect(await utils.parseIniFile('none')).toBe('none');
5769
expect(await utils.parseIniFile('php.ini-production')).toBe('production');
5870
expect(await utils.parseIniFile('php.ini-development')).toBe('development');
71+
expect(await utils.parseIniFile('/etc/php.ini-production')).toBe(
72+
'production'
73+
);
74+
expect(await utils.parseIniFile('/a-b/php.ini-development')).toBe(
75+
'development'
76+
);
5977
expect(await utils.parseIniFile('invalid')).toBe('production');
6078
});
6179

@@ -91,6 +109,22 @@ describe('Utils tests', () => {
91109
).toEqual(['apcu', 'mbstring', 'pdo_pgsql', 'posix', 'session']);
92110
});
93111

112+
it('checking shell helpers', () => {
113+
expect(utils.escapeForShell('a$b`c\\d"e', 'linux')).toBe(
114+
'a\\$b\\`c\\\\d\\"e'
115+
);
116+
expect(utils.escapeForShell('a$b`c"d', 'win32')).toBe('a`$b``c`"d');
117+
expect(utils.safeArg('vendor-pkg/repo@v1.0.0', 'linux')).toBe(
118+
'vendor-pkg/repo@v1.0.0'
119+
);
120+
expect(utils.safeArg('phpcs:>=3.0', 'linux')).toBe('"phpcs:>=3.0"');
121+
expect(utils.safeArg('foo$bar', 'win32')).toBe('"foo`$bar"');
122+
expect(utils.sanitizeShellInput('foo;$(`ls`)bar')).toBe('foolsbar');
123+
expect(utils.sanitizeShellInput('vendor/foo:1.*', true)).toBe(
124+
'vendor/foo:1.'
125+
);
126+
});
127+
94128
it('checking INIArray', async () => {
95129
expect(await utils.CSVArray('a=1, b=2, c=3')).toEqual([
96130
'a=1',
@@ -282,6 +316,9 @@ describe('Utils tests', () => {
282316
process.env['php-version'] = '8.2';
283317
expect(await utils.readPHPVersion()).toBe('8.2');
284318

319+
process.env['php-version'] = 'pre-installed';
320+
expect(await utils.readPHPVersion()).toBe('pre-installed');
321+
285322
delete process.env['php-version-file'];
286323
delete process.env['php-version'];
287324

@@ -291,7 +328,7 @@ describe('Utils tests', () => {
291328

292329
existsSync.mockReturnValue(true);
293330
readFileSync.mockReturnValue('setup-php');
294-
expect(await utils.readPHPVersion()).toBe('setup-php');
331+
await expect(utils.readPHPVersion()).rejects.toThrow('Invalid PHP version');
295332

296333
existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true);
297334
readFileSync.mockReturnValue(
@@ -312,6 +349,37 @@ describe('Utils tests', () => {
312349
readFileSync.mockClear();
313350
});
314351

352+
it('readPHPVersion rejects unsupported values from each source', async () => {
353+
const existsSync = jest.spyOn(fs, 'existsSync').mockImplementation();
354+
const readFileSync = jest.spyOn(fs, 'readFileSync').mockImplementation();
355+
356+
process.env['php-version'] = 'bogus';
357+
await expect(utils.readPHPVersion()).rejects.toThrow('php-version input');
358+
delete process.env['php-version'];
359+
360+
existsSync.mockReturnValue(true);
361+
readFileSync.mockReturnValue('bogus');
362+
await expect(utils.readPHPVersion()).rejects.toThrow('.php-version');
363+
364+
existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true);
365+
readFileSync.mockReturnValue('{"platform-overrides":{"php":"bogus"}}');
366+
await expect(utils.readPHPVersion()).rejects.toThrow(
367+
'composer.lock platform-overrides.php'
368+
);
369+
370+
existsSync
371+
.mockReturnValueOnce(false)
372+
.mockReturnValueOnce(false)
373+
.mockReturnValueOnce(true);
374+
readFileSync.mockReturnValue('{"config":{"platform":{"php":"bogus"}}}');
375+
await expect(utils.readPHPVersion()).rejects.toThrow(
376+
'composer.json config.platform.php'
377+
);
378+
379+
existsSync.mockClear();
380+
readFileSync.mockClear();
381+
});
382+
315383
it('checking setVariable', async () => {
316384
let script: string = await utils.setVariable('var', 'command', 'linux');
317385
expect(script).toEqual('\nvar="$(command)"\n');

‎dist/index.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/config.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export async function addINIValuesUnix(
1616
});
1717
return (
1818
'echo "' +
19-
ini_values.join('\n') +
19+
ini_values.map(v => utils.escapeForShell(v, 'linux')).join('\n') +
2020
'" | sudo tee -a "${pecl_file:-${ini_file[@]}}" >/dev/null 2>&1' +
2121
script
2222
);
@@ -37,7 +37,10 @@ export async function addINIValuesWindows(
3737
(await utils.addLog('$tick', line, 'Added to php.ini', 'win32')) + '\n';
3838
});
3939
return (
40-
'Add-Content "$php_dir\\php.ini" "' + ini_values.join('\n') + '"' + script
40+
'Add-Content "$php_dir\\php.ini" "' +
41+
ini_values.map(v => utils.escapeForShell(v, 'win32')).join('\n') +
42+
'"' +
43+
script
4144
);
4245
}
4346

‎src/install.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ export async function getScript(os: string): Promise<string> {
1818
const filename = os + (await utils.scriptExtension(os));
1919
const script_path = path.join(__dirname, '../src/scripts', filename);
2020
const run_path = script_path.replace(os, 'run');
21-
const extension_csv: string = await utils.getInput('extensions', false);
21+
const extension_csv: string = utils.sanitizeShellInput(
22+
await utils.getInput('extensions', false),
23+
true
24+
);
2225
const ini_values_csv: string = await utils.getInput('ini-values', false);
2326
const coverage_driver: string = await utils.getInput('coverage', false);
2427
const tools_csv: string = await utils.getInput('tools', false);
@@ -28,7 +31,7 @@ export async function getScript(os: string): Promise<string> {
2831
const ini_file: string = await utils.parseIniFile(
2932
await utils.getInput('ini-file', false)
3033
);
31-
let script = await utils.joins('.', script_path, version, ini_file);
34+
let script = await utils.joins('.', script_path, `'${version}'`, ini_file);
3235
if (extension_csv) {
3336
script += await extensions.addExtension(extension_csv, version, os);
3437
}

‎src/tools.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ export async function getVersion(
231231
case !!data.repository && major_minor_regex.test(data.version):
232232
return await getSemverVersion(data);
233233
default:
234-
return data.version.replace(/[><=^~]*/, '');
234+
return data.version.replace(/[^a-zA-Z0-9_.:@+,/-]/g, '');
235235
}
236236
}
237237

@@ -347,12 +347,9 @@ export async function addArchive(data: ToolData): Promise<string> {
347347
export async function addPackage(data: ToolData): Promise<string> {
348348
const command = await utils.getCommand(data.os, 'composer_tool');
349349
const parts: string[] = data.repository.split('/');
350-
const args: string = await utils.joins(
351-
parts[1],
352-
data.release,
353-
parts[0] + '/',
354-
data.scope
355-
);
350+
const args = [parts[1], data.release, parts[0] + '/', data.scope]
351+
.map(a => utils.safeArg(a, data.os))
352+
.join(' ');
356353
return command + args;
357354
}
358355

0 commit comments

Comments
 (0)