diff --git a/.changeset/silent-dodos-deny.md b/.changeset/silent-dodos-deny.md new file mode 100644 index 00000000..b7f6cc89 --- /dev/null +++ b/.changeset/silent-dodos-deny.md @@ -0,0 +1,5 @@ +--- +"simple-git": minor +--- + +Resolves potential command injection vulnerability by preventing use of `--upload-pack` in `git.fetch` diff --git a/simple-git/src/lib/tasks/fetch.ts b/simple-git/src/lib/tasks/fetch.ts index d5f57e1a..556e28aa 100644 --- a/simple-git/src/lib/tasks/fetch.ts +++ b/simple-git/src/lib/tasks/fetch.ts @@ -2,12 +2,23 @@ import { FetchResult } from '../../../typings'; import { parseFetchResult } from '../parsers/parse-fetch'; import { StringTask } from '../types'; -export function fetchTask(remote: string, branch: string, customArgs: string[]): StringTask { +import { configurationErrorTask, EmptyTask } from './task'; + +function disallowedCommand(command: string) { + return /^--upload-pack(=|$)/.test(command); +} + +export function fetchTask(remote: string, branch: string, customArgs: string[]): StringTask | EmptyTask { const commands = ['fetch', ...customArgs]; if (remote && branch) { commands.push(remote, branch); } + const banned = commands.find(disallowedCommand); + if (banned) { + return configurationErrorTask(`git.fetch: potential exploit argument blocked.`); + } + return { commands, format: 'utf-8', diff --git a/simple-git/test/unit/fetch.spec.ts b/simple-git/test/unit/fetch.spec.ts index c6583e4d..24451933 100644 --- a/simple-git/test/unit/fetch.spec.ts +++ b/simple-git/test/unit/fetch.spec.ts @@ -1,7 +1,8 @@ -import { assertExecutedCommands, closeWithSuccess, like, newSimpleGit } from './__fixtures__'; -import { SimpleGit } from "../../typings"; +import { promiseError } from '@kwsites/promise-result'; +import { assertExecutedCommands, assertGitError, closeWithSuccess, like, newSimpleGit } from './__fixtures__'; +import { SimpleGit } from '../../typings'; -describe('push', () => { +describe('fetch', () => { let git: SimpleGit; let callback: jest.Mock; @@ -58,4 +59,30 @@ describe('push', () => { assertExecutedCommands('fetch', '--all', '-v'); }); + + describe('failures', () => { + + it('disallows upload-pack as remote/branch', async () => { + const error = await promiseError(git.fetch('origin', '--upload-pack=touch ./foo')); + + assertGitError(error, 'potential exploit argument blocked'); + }); + + it('disallows upload-pack as varargs', async () => { + const error = await promiseError(git.fetch('origin', 'main', { + '--upload-pack': 'touch ./foo' + })); + + assertGitError(error, 'potential exploit argument blocked'); + }); + + it('disallows upload-pack as varargs', async () => { + const error = await promiseError(git.fetch('origin', 'main', [ + '--upload-pack', 'touch ./foo' + ])); + + assertGitError(error, 'potential exploit argument blocked'); + }); + + }) });