Skip to content

[Security] Fix command injection risk in cloudflared installer #7473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
gonzaloriestra wants to merge 1 commit into
base: main
Choose a base branch
from
+18 −11
Draft

[Security] Fix command injection risk in cloudflared installer #7473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 14 additions & 7 deletions packages/plugin-cloudflare/src/install-cloudflared.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import install, {CURRENT_CLOUDFLARE_VERSION, versionIsGreaterThan} from './install-cloudflared.js'
import * as fsActions from '@shopify/cli-kit/node/fs'
import * as http from '@shopify/cli-kit/node/http'
import * as system from '@shopify/cli-kit/node/system'
import {beforeEach, describe, expect, test, vi} from 'vitest'
import util from 'util'

import {WriteStream} from 'fs'
// eslint-disable-next-line no-restricted-imports
import * as childProcess from 'child_process'

vi.mock('child_process')
vi.mock('@shopify/cli-kit/node/system')
vi.mock('stream')

describe('install-cloudflare', () => {
beforeEach(() => {
vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({stdout: '', stderr: '', exitCode: 0})
vi.mocked(system.exec).mockResolvedValue(undefined)
vi.spyOn(util, 'promisify').mockReturnValue(vi.fn().mockReturnValue(Promise.resolve()))
vi.spyOn(http, 'fetch').mockReturnValue(Promise.resolve({ok: true, body: {pipe: vi.fn()}} as any))
vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(false)
Expand Down Expand Up @@ -98,9 +99,11 @@ describe('install-cloudflare', () => {
// Given
const env = {}
vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
vi.spyOn(childProcess, 'execFileSync').mockReturnValue(
`cloudflared version ${CURRENT_CLOUDFLARE_VERSION} (built 2023-03-13-1444 UTC)`,
)
vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({
stdout: `cloudflared version ${CURRENT_CLOUDFLARE_VERSION} (built 2023-03-13-1444 UTC)`,
stderr: '',
exitCode: 0,
})

// When
await install(env, 'win32', 'x64')
Expand All @@ -113,7 +116,11 @@ describe('install-cloudflare', () => {
// Given
const env = {}
vi.spyOn(fsActions, 'fileExistsSync').mockReturnValueOnce(true)
vi.spyOn(childProcess, 'execFileSync').mockReturnValue(`cloudflared version 2000.0.0 (built 2023-03-13-1444 UTC)`)
vi.mocked(system.captureOutputWithExitCode).mockResolvedValue({
stdout: `cloudflared version 2000.0.0 (built 2023-03-13-1444 UTC)`,
stderr: '',
exitCode: 0,
})

// When
await install(env, 'darwin', 'x64')
Expand Down
8 changes: 4 additions & 4 deletions packages/plugin-cloudflare/src/install-cloudflared.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ import {
unlinkFileSync,
createFileWriteStream,
} from '@shopify/cli-kit/node/fs'
import {captureOutputWithExitCode, exec} from '@shopify/cli-kit/node/system'
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'

export const CURRENT_CLOUDFLARE_VERSION = '2024.8.2'
const CLOUDFLARE_REPO = `https://github.com/cloudflare/cloudflared/releases/download/${CURRENT_CLOUDFLARE_VERSION}/`
Expand Down Expand Up @@ -82,7 +81,8 @@ export default async function install(env = process.env, platform = process.plat
if (fileExistsSync(binTarget)) {
// --version returns an string like "cloudflared version 2023.3.1 (built 2023-03-13-1444 UTC)"
try {
const versionArray = execFileSync(binTarget, ['--version'], {encoding: 'utf8'}).split(' ')
const {stdout} = await captureOutputWithExitCode(binTarget, ['--version'])
const versionArray = stdout.split(' ')
const versionNumber = versionArray.length > 2 ? versionArray[2] : '0.0.0'
const needsUpdate = versionIsGreaterThan(CURRENT_CLOUDFLARE_VERSION, versionNumber!)
if (!needsUpdate) {
Expand Down Expand Up @@ -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)})
await exec('tar', ['-xzf', filename], {cwd: dirname(binTarget)})
unlinkFileSync(`${binTarget}.tgz`)
await renameFile(`${dirname(binTarget)}/cloudflared`, binTarget)
}
Expand Down
Loading