Skip to content

Commit

Permalink
Prevent use of --upload-pack as a command in git.fetch to avoid p…
Browse files Browse the repository at this point in the history
…otential accidental command execution.
  • Loading branch information
steveukx committed Mar 11, 2022
1 parent e4ff627 commit d119ec4
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/silent-dodos-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"simple-git": minor
---

Resolves potential command injection vulnerability by preventing use of `--upload-pack` in `git.fetch`
13 changes: 12 additions & 1 deletion simple-git/src/lib/tasks/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<FetchResult> {
import { configurationErrorTask, EmptyTask } from './task';

function disallowedCommand(command: string) {
return /^--upload-pack(=|$)/.test(command);
}

export function fetchTask(remote: string, branch: string, customArgs: string[]): StringTask<FetchResult> | 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',
Expand Down
33 changes: 30 additions & 3 deletions simple-git/test/unit/fetch.spec.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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');
});

})
});

0 comments on commit d119ec4

Please sign in to comment.