diff --git a/packages/plugin-cloudflare/src/install-cloudflared.test.ts b/packages/plugin-cloudflare/src/install-cloudflared.test.ts index a72f73ae6db..5a7994257c3 100644 --- a/packages/plugin-cloudflare/src/install-cloudflared.test.ts +++ b/packages/plugin-cloudflare/src/install-cloudflared.test.ts @@ -42,10 +42,12 @@ describe('install-cloudflare', () => { const binPath = joinPath(tmpDir, 'cloudflared') const env = {SHOPIFY_CLI_CLOUDFLARED_PATH: binPath} mockFetch() - vi.mocked(childProcess.execSync).mockImplementation((_command, options) => { - // Simulate tar extracting the file - const cwd = options?.cwd as string - writeFileSync(joinPath(cwd, 'cloudflared'), 'extracted binary') + vi.mocked(childProcess.execFileSync).mockImplementation((command, args, options) => { + if (command === 'tar') { + // Simulate tar extracting the file + const cwd = options?.cwd as string + writeFileSync(joinPath(cwd, 'cloudflared'), 'extracted binary') + } return Buffer.from('') }) @@ -69,9 +71,11 @@ describe('install-cloudflare', () => { const binPath = joinPath(tmpDir, 'cloudflared') const env = {SHOPIFY_CLI_CLOUDFLARED_PATH: binPath} mockFetch() - vi.mocked(childProcess.execSync).mockImplementation((_command, options) => { - const cwd = options?.cwd as string - writeFileSync(joinPath(cwd, 'cloudflared'), 'extracted binary') + vi.mocked(childProcess.execFileSync).mockImplementation((command, args, options) => { + if (command === 'tar') { + const cwd = options?.cwd as string + writeFileSync(joinPath(cwd, 'cloudflared'), 'extracted binary') + } return Buffer.from('') }) @@ -88,6 +92,36 @@ describe('install-cloudflare', () => { }) }) + test.skipIf(process.platform === 'win32')( + 'installMacos is no longer vulnerable to command injection via binTarget', + async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + // A malicious path that attempts to escape the tar command and execute 'touch exploit' + const binPath = joinPath(tmpDir, '"; touch exploit; #') + const env = {SHOPIFY_CLI_CLOUDFLARED_PATH: binPath} + mockFetch() + vi.mocked(childProcess.execFileSync).mockImplementation((command, args, options) => { + if (command === 'tar') { + const cwd = options?.cwd as string + writeFileSync(joinPath(cwd, 'cloudflared'), 'extracted binary') + } + return Buffer.from('') + }) + + // When + await install(env, 'darwin', 'x64') + + // Then + expect(childProcess.execFileSync).toHaveBeenCalledWith( + 'tar', + expect.arrayContaining(['-xzf', expect.stringContaining('; touch exploit; #')]), + expect.anything(), + ) + }) + }, + ) + test('downloads the correct binary for linux', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given diff --git a/packages/plugin-cloudflare/src/install-cloudflared.ts b/packages/plugin-cloudflare/src/install-cloudflared.ts index 37664c67290..e772947aa7d 100644 --- a/packages/plugin-cloudflare/src/install-cloudflared.ts +++ b/packages/plugin-cloudflare/src/install-cloudflared.ts @@ -14,7 +14,7 @@ import {fileURLToPath} from 'url' import util from 'util' import {pipeline} from 'stream' // eslint-disable-next-line no-restricted-imports -import {execSync, execFileSync} from 'child_process' +import {execFileSync} from 'child_process' export const CURRENT_CLOUDFLARE_VERSION = '2024.8.2' const CLOUDFLARE_REPO = `https://github.com/cloudflare/cloudflared/releases/download/${CURRENT_CLOUDFLARE_VERSION}/` @@ -132,7 +132,7 @@ async function installWindows(file: string, binTarget: string) { async function installMacos(file: string, binTarget: string) { await downloadFile(file, `${binTarget}.tgz`) const filename = basename(`${binTarget}.tgz`) - execSync(`tar -xzf ${filename}`, {cwd: dirname(binTarget)}) + execFileSync('tar', ['-xzf', filename], {cwd: dirname(binTarget)}) unlinkFileSync(`${binTarget}.tgz`) await renameFile(`${dirname(binTarget)}/cloudflared`, binTarget) }