diff --git a/.changeset/wicked-files-vanish.md b/.changeset/wicked-files-vanish.md new file mode 100644 index 00000000..ab1327a5 --- /dev/null +++ b/.changeset/wicked-files-vanish.md @@ -0,0 +1,5 @@ +--- +"simple-git": minor +--- + +Resolves potential command injection vulnerability by preventing use of `--upload-pack` in `git.clone` diff --git a/simple-git/src/lib/tasks/clone.ts b/simple-git/src/lib/tasks/clone.ts index 74503501..d6b88ee9 100644 --- a/simple-git/src/lib/tasks/clone.ts +++ b/simple-git/src/lib/tasks/clone.ts @@ -1,10 +1,9 @@ -import { straightThroughStringTask } from './task'; +import { configurationErrorTask, EmptyTask, straightThroughStringTask } from './task'; import { OptionFlags, Options, StringTask } from '../types'; -import { append } from '../utils'; +import { append, filterString } from '../utils'; export type CloneOptions = Options & - OptionFlags< - '--bare' | + OptionFlags<'--bare' | '--dissociate' | '--mirror' | '--no-checkout' | @@ -15,25 +14,30 @@ export type CloneOptions = Options & '--remote-submodules' | '--single-branch' | '--shallow-submodules' | - '--verbose' - > & + '--verbose'> & OptionFlags<'--depth' | '-j' | '--jobs', number> & OptionFlags<'--branch' | '--origin' | '--recurse-submodules' | '--separate-git-dir' | '--shallow-exclude' | '--shallow-since' | '--template', string> -export function cloneTask(repo: string | undefined, directory: string | undefined, customArgs: string[]): StringTask { +function disallowedCommand(command: string) { + return /^--upload-pack(=|$)/.test(command); +} + +export function cloneTask(repo: string | undefined, directory: string | undefined, customArgs: string[]): StringTask | EmptyTask { const commands = ['clone', ...customArgs]; - if (typeof repo === 'string') { - commands.push(repo); - } - if (typeof directory === 'string') { - commands.push(directory); + + filterString(repo) && commands.push(repo); + filterString(directory) && commands.push(directory); + + const banned = commands.find(disallowedCommand); + if (banned) { + return configurationErrorTask(`git.fetch: potential exploit argument blocked.`); } return straightThroughStringTask(commands); } -export function cloneMirrorTask(repo: string | undefined, directory: string | undefined, customArgs: string[]): StringTask { - append(customArgs,'--mirror'); +export function cloneMirrorTask(repo: string | undefined, directory: string | undefined, customArgs: string[]) { + append(customArgs, '--mirror'); return cloneTask(repo, directory, customArgs); } diff --git a/simple-git/test/unit/clone.spec.ts b/simple-git/test/unit/clone.spec.ts index 6fd3a554..c52a4ea5 100644 --- a/simple-git/test/unit/clone.spec.ts +++ b/simple-git/test/unit/clone.spec.ts @@ -1,5 +1,6 @@ +import { promiseError } from '@kwsites/promise-result'; import { SimpleGit, TaskOptions } from 'typings'; -import { assertExecutedCommands, closeWithSuccess, newSimpleGit } from './__fixtures__'; +import { assertExecutedCommands, assertGitError, closeWithSuccess, newSimpleGit } from './__fixtures__'; describe('clone', () => { let git: SimpleGit; @@ -15,7 +16,7 @@ describe('clone', () => { beforeEach(() => git = newSimpleGit()); - it.each(cloneTests)('callbacks - %s %s', async (api, name, cloneArgs, executedCommands)=> { + it.each(cloneTests)('callbacks - %s %s', async (api, name, cloneArgs, executedCommands) => { const callback = jest.fn(); const queue = (git[api] as any)(...cloneArgs, callback); await closeWithSuccess(name); @@ -32,5 +33,30 @@ describe('clone', () => { expect(await queue).toBe(name); assertExecutedCommands(...executedCommands); }); + + describe('failures', () => { + + it('disallows upload-pack as remote/branch', async () => { + const error = await promiseError(git.clone('origin', '--upload-pack=touch ./foo')); + + assertGitError(error, 'potential exploit argument blocked'); + }); + + it('disallows upload-pack as varargs', async () => { + const error = await promiseError(git.clone('origin', 'main', { + '--upload-pack': 'touch ./foo' + })); + + assertGitError(error, 'potential exploit argument blocked'); + }); + + it('disallows upload-pack as varargs', async () => { + const error = await promiseError(git.clone('origin', 'main', [ + '--upload-pack', 'touch ./foo' + ])); + + assertGitError(error, 'potential exploit argument blocked'); + }); + }); });