GHSA-pqwm-q9pv-ph8r - Fix CWE-78 [skip ci]

Co-Authored-By: maki <amarrec@quarkslab.com>

__tests__/config.test.ts

@@ -13,4 +13,17 @@ describe('Config tests', () => {
   `('checking addINIValues on $os', async ({ini_values, os, output}) => {
     expect(await config.addINIValues(ini_values, os)).toContain(output);
   });
+
+  it.each`
+    ini_values                           | os         | output
+    ${'disable_functions="exec,system"'} | ${'linux'} | ${'echo "disable_functions=exec,system" | sudo tee -a "${pecl_file:-${ini_file[@]}}"'}
+    ${'disable_functions="exec,system"'} | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "disable_functions=exec,system"'}
+    ${'a=$(id)'}                         | ${'linux'} | ${'echo "a=\'\\$(id)\'"'}
+    ${'a=$(id)'}                         | ${'win32'} | ${'Add-Content "$php_dir\\php.ini" "a=\'`$(id)\'"'}
+  `(
+    'addINIValues survives quoted values and escapes shell metas: $ini_values, $os',
+    async ({ini_values, os, output}) => {
+      expect(await config.addINIValues(ini_values, os)).toContain(output);
+    }
+  );
 });

__tests__/tools.test.ts

@@ -187,6 +187,7 @@ describe('Tools tests', () => {
     ${'1.2.3-dev'}     | ${'tool'}     | ${'phar'}     | ${'1.2.3-dev'}
     ${'1.2.3-alpha1'}  | ${'tool'}     | ${'phar'}     | ${'1.2.3-alpha1'}
     ${'1.2.3-alpha.1'} | ${'tool'}     | ${'phar'}     | ${'1.2.3-alpha.1'}
+    ${'1.>=0'}         | ${'tool'}     | ${'phar'}     | ${'1.0'}
   `(
     'checking getVersion: $version, $tool, $type',
     async ({version, tool, type, expected}) => {
@@ -321,6 +322,28 @@ describe('Tools tests', () => {
     expect(await tools.addPackage(data)).toContain(script);
   });
 
+  it.each`
+    release           | os         | expected
+    ${'tool:>=1.2'}   | ${'linux'} | ${'add_composer_tool tool "tool:>=1.2" user/ global'}
+    ${'tool:1.*'}     | ${'linux'} | ${'add_composer_tool tool "tool:1.*" user/ global'}
+    ${'tool:>=1.2'}   | ${'win32'} | ${'Add-ComposerTool tool "tool:>=1.2" user/ global'}
+    ${'psalm:^5||^6'} | ${'linux'} | ${'add_composer_tool tool "psalm:^5||^6" user/ global'}
+    ${'psalm:>=5,<6'} | ${'linux'} | ${'add_composer_tool tool "psalm:>=5,<6" user/ global'}
+  `(
+    'addPackage quotes constraint operators: $release, $os',
+    async ({release, os, expected}) => {
+      const data = getData({
+        tool: 'tool',
+        version: '>=1.2',
+        repository: 'user/tool',
+        os: os,
+        scope: 'global'
+      });
+      data['release'] = release;
+      expect(await tools.addPackage(data)).toContain(expected);
+    }
+  );
+
   it.each`
     version     | php_version | os          | script
     ${'latest'} | ${'8.0'}    | ${'linux'}  | ${'add_tool https://github.com/phar-io/phive/releases/download/3.2.1/phive-3.2.1.phar phive'}
@@ -651,7 +674,7 @@ describe('Tools tests', () => {
         'add_devtools phpize',
         'add_tool https://github.com/phpmd/phpmd/releases/latest/download/phpmd.phar phpmd "--version"',
         'add_tool https://github.com/phpspec/phpspec/releases/latest/download/phpspec.phar phpspec "-V"',
-        'add_composer_tool phpunit-bridge phpunit-bridge:5.6.* symfony/ global',
+        'add_composer_tool phpunit-bridge "phpunit-bridge:5.6.*" symfony/ global',
         'add_composer_tool phpunit-polyfills phpunit-polyfills:1.0.1 yoast/ global',
         'add_protoc 1.2.3',
         'add_tool https://github.com/vimeo/psalm/releases/latest/download/psalm.phar psalm "-v"',
@@ -711,7 +734,7 @@ describe('Tools tests', () => {
         'Add-ComposerTool codeception codeception codeception/ global',
         'Add-ComposerTool prestissimo prestissimo hirak/ global',
         'Add-ComposerTool automatic-composer-prefetcher automatic-composer-prefetcher narrowspark/ global',
-        'Add-ComposerTool phinx phinx:1.2.* robmorgan/ scoped',
+        'Add-ComposerTool phinx "phinx:1.2.*" robmorgan/ scoped',
         'Add-ComposerTool phinx phinx:^1.2 robmorgan/ global',
         'Add-ComposerTool tool tool:1.2.3 user/ global',
         'Add-ComposerTool tool tool:~1.2 user/ global'

__tests__/utils.test.ts

@@ -50,12 +50,37 @@ describe('Utils tests', () => {
     expect(fetchSpy).toHaveBeenCalledTimes(2);
   });
 
+  it('parseVersion rejects unsupported inputs', async () => {
+    await expect(utils.parseVersion('foo')).rejects.toThrow(
+      'Invalid PHP version:'
+    );
+    await expect(utils.parseVersion('8.x.1')).rejects.toThrow(
+      'Invalid PHP version:'
+    );
+  });
+
+  it('parseVersion rejects unexpected manifest values', async () => {
+    const fetchSpy = jest
+      .spyOn(fetchModule, 'fetch')
+      .mockResolvedValue({data: '{ "latest": "8.1.0" }'});
+    await expect(utils.parseVersion('latest')).rejects.toThrow(
+      'Invalid PHP version in manifest:'
+    );
+    fetchSpy.mockRestore();
+  });
+
   it('checking parseIniFile', async () => {
     expect(await utils.parseIniFile('production')).toBe('production');
     expect(await utils.parseIniFile('development')).toBe('development');
     expect(await utils.parseIniFile('none')).toBe('none');
     expect(await utils.parseIniFile('php.ini-production')).toBe('production');
     expect(await utils.parseIniFile('php.ini-development')).toBe('development');
+    expect(await utils.parseIniFile('/etc/php.ini-production')).toBe(
+      'production'
+    );
+    expect(await utils.parseIniFile('/a-b/php.ini-development')).toBe(
+      'development'
+    );
     expect(await utils.parseIniFile('invalid')).toBe('production');
   });
 
@@ -91,6 +116,40 @@ describe('Utils tests', () => {
     ).toEqual(['apcu', 'mbstring', 'pdo_pgsql', 'posix', 'session']);
   });
 
+  it('checking escapeForShell', () => {
+    expect(utils.escapeForShell('plain', 'linux')).toBe('plain');
+    expect(utils.escapeForShell('a"b', 'linux')).toBe('a\\"b');
+    expect(utils.escapeForShell('a$b`c\\d"e', 'linux')).toBe(
+      'a\\$b\\`c\\\\d\\"e'
+    );
+    expect(utils.escapeForShell('a$b`c"d', 'win32')).toBe('a`$b``c`"d');
+  });
+
+  it('checking safeArg', () => {
+    expect(utils.safeArg('plain', 'linux')).toBe('plain');
+    expect(utils.safeArg('vendor-pkg/repo@v1.0.0', 'linux')).toBe(
+      'vendor-pkg/repo@v1.0.0'
+    );
+    expect(utils.safeArg('phpcs:>=3.0', 'linux')).toBe('"phpcs:>=3.0"');
+    expect(utils.safeArg('vendor/foo:1.*', 'linux')).toBe('"vendor/foo:1.*"');
+    expect(utils.safeArg('foo$bar', 'linux')).toBe('"foo\\$bar"');
+    expect(utils.safeArg('foo$bar', 'win32')).toBe('"foo`$bar"');
+    expect(utils.safeArg('', 'linux')).toBe('');
+  });
+
+  it('checking sanitizeShellInput', () => {
+    expect(utils.sanitizeShellInput('mbstring, intl')).toBe('mbstring, intl');
+    expect(utils.sanitizeShellInput('foo;ls;bar')).toBe('foolsbar');
+    expect(utils.sanitizeShellInput('foo$(id)bar')).toBe('fooidbar');
+    expect(utils.sanitizeShellInput('a`b`c\\d"e\'f')).toBe('abcdef');
+    expect(utils.sanitizeShellInput('vendor/foo:1.*', true)).toBe(
+      'vendor/foo:1.'
+    );
+    expect(utils.sanitizeShellInput('vendor/foo@feat?ure', true)).toBe(
+      'vendor/foo@feature'
+    );
+  });
+
   it('checking INIArray', async () => {
     expect(await utils.CSVArray('a=1, b=2, c=3')).toEqual([
       'a=1',
@@ -291,7 +350,7 @@ describe('Utils tests', () => {
 
     existsSync.mockReturnValue(true);
     readFileSync.mockReturnValue('setup-php');
-    expect(await utils.readPHPVersion()).toBe('setup-php');
+    await expect(utils.readPHPVersion()).rejects.toThrow('Invalid PHP version');
 
     existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true);
     readFileSync.mockReturnValue(
@@ -312,6 +371,45 @@ describe('Utils tests', () => {
     readFileSync.mockClear();
   });
 
+  it('readPHPVersion rejects unsupported values from each source', async () => {
+    const existsSync = jest.spyOn(fs, 'existsSync').mockImplementation();
+    const readFileSync = jest.spyOn(fs, 'readFileSync').mockImplementation();
+
+    process.env['php-version'] = 'bogus';
+    await expect(utils.readPHPVersion()).rejects.toThrow(
+      'Invalid PHP version in php-version input'
+    );
+    delete process.env['php-version'];
+
+    existsSync.mockReturnValue(true);
+    readFileSync.mockReturnValue('bogus');
+    await expect(utils.readPHPVersion()).rejects.toThrow(
+      'Invalid PHP version in .php-version'
+    );
+
+    existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true);
+    readFileSync.mockReturnValue(
+      JSON.stringify({'platform-overrides': {php: 'bogus'}})
+    );
+    await expect(utils.readPHPVersion()).rejects.toThrow(
+      'Invalid PHP version in composer.lock platform-overrides.php'
+    );
+
+    existsSync
+      .mockReturnValueOnce(false)
+      .mockReturnValueOnce(false)
+      .mockReturnValueOnce(true);
+    readFileSync.mockReturnValue(
+      JSON.stringify({config: {platform: {php: 'bogus'}}})
+    );
+    await expect(utils.readPHPVersion()).rejects.toThrow(
+      'Invalid PHP version in composer.json config.platform.php'
+    );
+
+    existsSync.mockClear();
+    readFileSync.mockClear();
+  });
+
   it('checking setVariable', async () => {
     let script: string = await utils.setVariable('var', 'command', 'linux');
     expect(script).toEqual('\nvar="$(command)"\n');

src/config.ts

@@ -16,7 +16,7 @@ export async function addINIValuesUnix(
   });
   return (
     'echo "' +
-    ini_values.join('\n') +
+    ini_values.map(v => utils.escapeForShell(v, 'linux')).join('\n') +
     '" | sudo tee -a "${pecl_file:-${ini_file[@]}}" >/dev/null 2>&1' +
     script
   );
@@ -37,7 +37,10 @@ export async function addINIValuesWindows(
       (await utils.addLog('$tick', line, 'Added to php.ini', 'win32')) + '\n';
   });
   return (
-    'Add-Content "$php_dir\\php.ini" "' + ini_values.join('\n') + '"' + script
+    'Add-Content "$php_dir\\php.ini" "' +
+    ini_values.map(v => utils.escapeForShell(v, 'win32')).join('\n') +
+    '"' +
+    script
   );
 }
 

src/install.ts

@@ -18,7 +18,10 @@ export async function getScript(os: string): Promise<string> {
   const filename = os + (await utils.scriptExtension(os));
   const script_path = path.join(__dirname, '../src/scripts', filename);
   const run_path = script_path.replace(os, 'run');
-  const extension_csv: string = await utils.getInput('extensions', false);
+  const extension_csv: string = utils.sanitizeShellInput(
+    await utils.getInput('extensions', false),
+    true
+  );
   const ini_values_csv: string = await utils.getInput('ini-values', false);
   const coverage_driver: string = await utils.getInput('coverage', false);
   const tools_csv: string = await utils.getInput('tools', false);
@@ -28,7 +31,7 @@ export async function getScript(os: string): Promise<string> {
   const ini_file: string = await utils.parseIniFile(
     await utils.getInput('ini-file', false)
   );
-  let script = await utils.joins('.', script_path, version, ini_file);
+  let script = await utils.joins('.', script_path, `'${version}'`, ini_file);
   if (extension_csv) {
     script += await extensions.addExtension(extension_csv, version, os);
   }

src/scripts/darwin.sh

@@ -300,6 +300,10 @@ export HOMEBREW_NO_INSTALL_FROM_API=1
 
 # shellcheck source=.
 . "${scripts:?}"/unix.sh
+if ! [[ "$version" =~ ^[0-9]+\.[0-9]+$ ]]; then
+  fail_fast=true
+  add_log "${cross:?}" "PHP" "Invalid PHP version: $version"
+fi
 . "${scripts:?}"/tools/brew.sh
 . "${scripts:?}"/tools/retry.sh
 . "${scripts:?}"/tools/add_tools.sh

src/scripts/linux.sh

@@ -344,6 +344,10 @@ link_apt_fast >/dev/null 2>&1
 . /etc/os-release
 # shellcheck source=.
 . "${scripts:?}"/unix.sh
+if ! [[ "$version" =~ ^[0-9]+\.[0-9]+$ ]]; then
+  fail_fast=true
+  add_log "${cross:?}" "PHP" "Invalid PHP version: $version"
+fi
 . "${scripts:?}"/tools/ppa.sh
 . "${scripts:?}"/tools/add_tools.sh
 . "${scripts:?}"/extensions/source.sh

src/scripts/win32.ps1

@@ -1,7 +1,6 @@
 param (
   [Parameter(Position = 0, Mandatory = $true)]
-  [ValidateNotNull()]
+  [ValidatePattern('^\d+\.\d+$')]
-  [ValidateLength(1, [int]::MaxValue)]
   [string]
   $version = '8.4',
   [Parameter(Position = 1, Mandatory = $true)]

src/tools.ts

@@ -231,7 +231,7 @@ export async function getVersion(
     case !!data.repository && major_minor_regex.test(data.version):
       return await getSemverVersion(data);
     default:
-      return data.version.replace(/[><=^~]*/, '');
+      return data.version.replace(/[^a-zA-Z0-9_.:@+,/-]/g, '');
   }
 }
 
@@ -347,12 +347,9 @@ export async function addArchive(data: ToolData): Promise<string> {
 export async function addPackage(data: ToolData): Promise<string> {
   const command = await utils.getCommand(data.os, 'composer_tool');
   const parts: string[] = data.repository.split('/');
-  const args: string = await utils.joins(
+  const args = [parts[1], data.release, parts[0] + '/', data.scope]
-    parts[1],
+    .map(a => utils.safeArg(a, data.os))
-    data.release,
+    .join(' ');
-    parts[0] + '/',
-    data.scope
-  );
   return command + args;
 }
 

src/utils.ts

@@ -66,11 +66,20 @@ export async function parseVersion(version: string): Promise<string> {
       for (const manifestURL of await getManifestURLS()) {
         const fetchResult = await fetch.fetch(manifestURL);
         if (fetchResult['data'] ?? false) {
-          return JSON.parse(fetchResult['data'])[version];
+          const resolved: string = JSON.parse(fetchResult['data'])[version];
+          if (resolved !== undefined && !/^\d+\.\d+$/.test(resolved)) {
+            throw new Error(
+              `Invalid PHP version in manifest: ${resolved.slice(0, 10)}`
+            );
+          }
+          return resolved;
         }
       }
       throw new Error(`Could not fetch the PHP version manifest.`);
     default:
+      if (!/^\d+(\.\d+){0,2}$/.test(version)) {
+        throw new Error(`Invalid PHP version: ${version.slice(0, 20)}`);
+      }
       switch (true) {
         case version.length > 1:
           return version.slice(0, 3);
@@ -86,14 +95,11 @@ export async function parseVersion(version: string): Promise<string> {
  * @param ini_file
  */
 export async function parseIniFile(ini_file: string): Promise<string> {
-  switch (true) {
+  if (/^(production|development|none)$/.test(ini_file)) {
-    case /^(production|development|none)$/.test(ini_file):
     return ini_file;
-    case /php\.ini-(production|development)$/.test(ini_file):
-      return ini_file.split('-')[1];
-    default:
-      return 'production';
   }
+  const match = ini_file.match(/php\.ini-(production|development)$/);
+  return match ? match[1] : 'production';
 }
 
 /**
@@ -172,10 +178,10 @@ export async function log(
 export async function stepLog(message: string, os: string): Promise<string> {
   switch (os) {
     case 'win32':
-      return 'Step-Log "' + message + '"';
+      return 'Step-Log "' + escapeForShell(message, os) + '"';
     case 'linux':
     case 'darwin':
-      return 'step_log "' + message + '"';
+      return 'step_log "' + escapeForShell(message, os) + '"';
     default:
       return await log('Platform ' + os + ' is not supported', os, 'error');
   }
@@ -196,15 +202,52 @@ export async function addLog(
 ): Promise<string> {
   switch (os) {
     case 'win32':
-      return 'Add-Log "' + mark + '" "' + subject + '" "' + message + '"';
+      return (
+        'Add-Log "' +
+        mark +
+        '" "' +
+        escapeForShell(subject, os) +
+        '" "' +
+        escapeForShell(message, os) +
+        '"'
+      );
     case 'linux':
     case 'darwin':
-      return 'add_log "' + mark + '" "' + subject + '" "' + message + '"';
+      return (
+        'add_log "' +
+        mark +
+        '" "' +
+        escapeForShell(subject, os) +
+        '" "' +
+        escapeForShell(message, os) +
+        '"'
+      );
     default:
       return await log('Platform ' + os + ' is not supported', os, 'error');
   }
 }
 
+export function escapeForShell(value: string, os: string): string {
+  if (os === 'win32') {
+    return value.replace(/[`$"]/g, '`$&');
+  }
+  return value.replace(/[\\`$"]/g, '\\$&');
+}
+
+export function safeArg(value: string, os: string): string {
+  if (!/^[a-zA-Z0-9_./:@+,~^-]*$/.test(value)) {
+    return '"' + escapeForShell(value, os) + '"';
+  }
+  return value;
+}
+
+export function sanitizeShellInput(value: string, strict = false): string {
+  const pattern = strict
+    ? /[$`"';|&(){}[\]\\<>*?\n\r\t]/g
+    : /[$`"';|&(){}[\]\\\n\r\t]/g;
+  return value.replace(pattern, '');
+}
+
 /**
  * Function to break extension csv into an array
  *
@@ -431,12 +474,24 @@ export async function parseExtensionSource(
   );
 }
 
+const VERSION_INPUT_REGEX =
+  /^(latest|lowest|highest|nightly|master|\d+\.x|\d+(\.\d+){0,2})$/;
+
+function validatePHPVersionInput(version: string, source: string): void {
+  if (!VERSION_INPUT_REGEX.test(version)) {
+    throw new Error(
+      `Invalid PHP version in ${source}: ${version.slice(0, 20)}`
+    );
+  }
+}
+
 /**
  * Read php version from input or file
  */
 export async function readPHPVersion(): Promise<string> {
   const version = await getInput('php-version', false);
   if (version) {
+    validatePHPVersionInput(version, 'php-version input');
     return version;
   }
   const versionFile =
@@ -446,7 +501,9 @@ export async function readPHPVersion(): Promise<string> {
     const match: RegExpMatchArray | null = contents.match(
       /^(?:php\s)?(\d+\.\d+\.\d+)$/m
     );
-    return match ? match[1] : contents.trim();
+    const raw = match ? match[1] : contents.trim();
+    validatePHPVersionInput(raw, versionFile);
+    return raw;
   } else if (versionFile !== '.php-version') {
     throw new Error(`Could not find '${versionFile}' file.`);
   }
@@ -460,7 +517,9 @@ export async function readPHPVersion(): Promise<string> {
       lockFileContents['platform-overrides'] &&
       lockFileContents['platform-overrides']['php']
     ) {
-      return lockFileContents['platform-overrides']['php'];
+      const raw: string = lockFileContents['platform-overrides']['php'];
+      validatePHPVersionInput(raw, 'composer.lock platform-overrides.php');
+      return raw;
     }
   }
 
@@ -475,7 +534,9 @@ export async function readPHPVersion(): Promise<string> {
       composerFileContents['config']['platform'] &&
       composerFileContents['config']['platform']['php']
     ) {
-      return composerFileContents['config']['platform']['php'];
+      const raw: string = composerFileContents['config']['platform']['php'];
+      validatePHPVersionInput(raw, 'composer.json config.platform.php');
+      return raw;
     }
   }