Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions .changeset/safe-rebase-exec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@simple-git/argv-parser": patch
"simple-git": patch
---

Add `allowUnsafeExec` detection to `rebase -x` and `rebase --exec`.

Thanks to @gdegrange for the vulnerability report.
7 changes: 7 additions & 0 deletions .changeset/safe-trailer-config.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@simple-git/argv-parser": patch
---

Add `allowUnsafeCommandBinaries` detection to configuring `trailer.<token>.cmd` and `trailer.<token>.command`.

Thanks to @sec-reex for the vulnerability report.
21 changes: 21 additions & 0 deletions docs/PLUGIN-UNSAFE-ACTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ await simpleGit({ unsafe: { allowAbbreviatedOptions: true } })
.raw('fetch', '--conf=user.name=me');
```

### Command Binaries

Options that allow supplying the path to executable binaries and writing to configuration options that set the path
to executable binaries are caught as potentially unsafe RCE vectors, enabled through the
`allowUnsafeCommandBinaries` option.

```typescript
import { simpleGit } from 'simple-git';

// throws the GitPluginError for allowUnsafeCommandBinaries
await simpleGit().raw('rebase', '--exec', '/custom/path');
await simpleGit().raw('config', 'trailer.foo.command', '/custom/path');

// opt in to using custom paths to executable binaries
await simpleGit({ unsafe: { allowUnsafeCommandBinaries: true } })
.raw('config', 'trailer.foo.command', '/custom/path')
.raw('rebase', '--exec', '/custom/path');
```

### Custom upload and receive packs

Instead of using the default `git-receive-pack` and `git-upload-pack` binaries to parse incoming and outgoing
Expand Down Expand Up @@ -478,6 +497,8 @@ await simpleGit({ unsafe: { allowUnsafeExec: true } })
.raw('--exec-path=/opt/custom/libexec/git-core', 'ls-remote', 'https://example.com/repo.git');
```

The `allowUnsafeExec` category also applies to `--exec` or `-x` options when rebasing.

### Environment-based configuration

Git supports injecting configuration values at runtime through a set of numbered environment variables:
Expand Down
16 changes: 16 additions & 0 deletions packages/argv-parser/src/tokens/flag-specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ const COMMANDS: Record<string, FlagSpec> = {
short: new Map(),
long: new Set(['exec', 'receive-pack']),
},
rebase: {
short: new Map([
['X', true], // -X <option> strategy option
['f', false], // -f force-rebase
['i', false], // -i interactive
['k', false], // -k keep-base
['m', false], // -m merge
['n', false], // -n no-stat
['q', false], // -q quiet
['r', false], // -r rebase-merges
['s', true], // -s <strategy>
['v', false], // -v verbose
['x', true], // -x <cmd> exec
]),
long: new Set(['exec', 'onto', 'strategy', 'strategy-option']),
},
};

const EMPTY: FlagSpec = { short: new Map(), long: new Set() };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,7 @@ const preventUnsafeConfig = [
preventConfigBuilder('sequence.editor', 'allowUnsafeEditor'),
preventExpandedConfigBuilder('submodule.update', 'allowUnsafeSubmodule'),
preventExpandedConfigBuilder('tar.command', 'allowUnsafeCommandBinaries'),
preventExpandedConfigBuilder('trailer.cmd', 'allowUnsafeCommandBinaries'),
preventExpandedConfigBuilder('trailer.command', 'allowUnsafeCommandBinaries'),
preventExpandedConfigBuilder('url.insteadOf', 'allowUnsafeUrlRewrite'),
];
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ const preventUnsafeFlags = [
preventFlagBuilder('clone', /^-\w*u/, 'allowUnsafePack'),
preventFlagBuilder('clone', '--u', 'allowUnsafePack'),
preventFlagBuilder('push', /^--exec$/, 'allowUnsafePack', { name: '--exec' }),
// `git` accepts unambiguous abbreviations of long options, so `--ex` and `--exe` are `--exec`
preventFlagBuilder('rebase', /^(-x|--ex(ec?)?)$/, 'allowUnsafeExec', { name: '-x or --exec' }),
preventFlagBuilder(null, '--template', 'allowUnsafeTemplateDir'),
preventFlagBuilder(null, '--exec-path', 'allowUnsafeExec', pathTakingGlobal),
// `git` reads the configuration of whichever repository these name, so the
Expand Down
3 changes: 3 additions & 0 deletions packages/argv-parser/test/vulnerability-analysis.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ describe('VulnerabilityAnalysis', () => {
['submodule.evil.update', '!malicious', 'allowUnsafeSubmodule'],
['tar.command', '!malicious', 'allowUnsafeCommandBinaries'],
['tar.foo.command', '!malicious', 'allowUnsafeCommandBinaries'],
['trailer.foo.command', '!malicious', 'allowUnsafeCommandBinaries'],
['trailer.foo.cmd', '!malicious', 'allowUnsafeCommandBinaries'],
['trailer.cmd', '!malicious', 'allowUnsafeCommandBinaries'],
['url.https://evil.com.insteadOf', 'https://github.com', 'allowUnsafeUrlRewrite'],
['url.https://evil.com.insteadOf', 'git@github.com:', 'allowUnsafeUrlRewrite'],
])('writing %s = %s to the git config', (key, value, category) => {
Expand Down
35 changes: 35 additions & 0 deletions simple-git/test/unit/vulnerabilities/rebase-exec.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { promiseError } from '@kwsites/promise-result';
import { describe, expect, it } from 'vitest';

import { assertGitError, closeWithSuccess } from '../__fixtures__';
import { simpleGit } from '../../../src/lib/git-factory';

describe('rebase --exec', () => {
it('catches bare rebase -x', async () => {
const task = promiseError(simpleGit().rebase(['-x', 'cmd']));
await promiseError(closeWithSuccess(''));

assertGitError(await task, 'allowUnsafeExec');
});

it('catches rebase -qx', async () => {
const task = promiseError(simpleGit().rebase(['-qx', 'cmd']));
await promiseError(closeWithSuccess(''));

assertGitError(await task, 'allowUnsafeExec');
});

it('catches rebase --exec', async () => {
const task = promiseError(simpleGit().rebase(['--exec', 'cmd']));
await promiseError(closeWithSuccess(''));

assertGitError(await task, 'allowUnsafeExec');
});

it('allows: rebase -qm', async () => {
const task = promiseError(simpleGit().rebase(['-qm', 'foo']));
await promiseError(closeWithSuccess(''));

expect(await task).toBeUndefined();
});
});
Loading