Skip to content

Commit 1bb14df

Browse files
steveukxsteveukx
authored
Merge pull request #1163 from steveukx/claude/add-git-config-vulnerabilities-Aowhg
feat(security): add GIT_CONFIG_PARAMETERS env var and new vulnerable …
1 parent 0e485f7 commit 1bb14df

10 files changed

Lines changed: 964 additions & 946 deletions

File tree

‎.changeset/hungry-kings-love.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+
Vulnerability detection expanded to include `pager.*`, `uploadpack.packObjectsHook`, `difftool.*.cmd` and use of the `GIT_CONFIG_PARAMETERS` environment variable
6+
7+
Thanks to @threalwinky and @nuc13us for identifying.

‎.yarn/releases/yarn-4.12.0.cjs‎

Lines changed: 0 additions & 942 deletions
This file was deleted.

‎.yarn/releases/yarn-4.14.1.cjs‎

Lines changed: 940 additions & 0 deletions
Large diffs are not rendered by default.

‎.yarnrc.yml‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1+
approvedGitRepositories: []
2+
13
compressionLevel: mixed
24

35
enableGlobalCache: false
46

7+
enableScripts: false
8+
59
nodeLinker: node-modules
610

711
npmPublishAccess: public
812

9-
npmPublishRegistry: https://registry.npmjs.org
13+
npmPublishRegistry: "https://registry.npmjs.org"
1014

11-
yarnPath: .yarn/releases/yarn-4.12.0.cjs
15+
yarnPath: .yarn/releases/yarn-4.14.1.cjs

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,5 @@
3232
"@biomejs/biome": "^2.1.4",
3333
"@kwsites/exec-p": "^0.4.0"
3434
},
35-
"packageManager": "yarn@4.12.0"
35+
"packageManager": "yarn@4.14.1"
3636
}

‎packages/argv-parser/src/env/parse-env.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const GitEnvKeys = {
88
'git_config_global': 'allowUnsafeConfigPaths',
99
'git_config_system': 'allowUnsafeConfigPaths',
1010
'git_config_count': 'allowUnsafeConfigEnvCount',
11+
'git_config_parameters': 'allowUnsafeConfigEnvCount',
1112
'git_config': 'allowUnsafeConfigPaths',
1213
'git_editor': 'allowUnsafeEditor',
1314
'git_exec_path': 'allowUnsafeConfigPaths',

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,16 +48,19 @@ const preventUnsafeConfig = [
4848
preventExpandedConfigBuilder('credential.helper', 'allowUnsafeCredentialHelper'),
4949
preventExpandedConfigBuilder('diff.command', 'allowUnsafeDiffExternal'),
5050
preventConfigBuilder('diff.external', 'allowUnsafeDiffExternal'),
51+
preventExpandedConfigBuilder('difftool.cmd', 'allowUnsafeDiffExternal'),
5152
preventExpandedConfigBuilder('diff.textconv', 'allowUnsafeDiffTextConv'),
5253
preventExpandedConfigBuilder('filter.clean', 'allowUnsafeFilter'),
5354
preventExpandedConfigBuilder('filter.smudge', 'allowUnsafeFilter'),
5455
preventExpandedConfigBuilder('gpg.program', 'allowUnsafeGpgProgram'),
5556
preventConfigBuilder('init.templateDir', 'allowUnsafeTemplateDir'),
57+
preventExpandedConfigBuilder('pager.', 'allowUnsafePager'),
5658
preventExpandedConfigBuilder('merge.driver', 'allowUnsafeMergeDriver'),
5759
preventExpandedConfigBuilder('mergetool.path', 'allowUnsafeMergeDriver'),
5860
preventExpandedConfigBuilder('mergetool.cmd', 'allowUnsafeMergeDriver'),
5961
preventExpandedConfigBuilder('protocol.allow', 'allowUnsafeProtocolOverride'),
6062
preventExpandedConfigBuilder('remote.receivepack', 'allowUnsafePack'),
6163
preventExpandedConfigBuilder('remote.uploadpack', 'allowUnsafePack'),
64+
preventConfigBuilder('uploadpack.packObjectsHook', 'allowUnsafePack'),
6265
preventConfigBuilder('sequence.editor', 'allowUnsafeEditor'),
6366
];

‎packages/argv-parser/test/parse-env.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ describe('parseEnv', () => {
2828
['GIT_CONFIG_GLOBAL', '/tmp/malicious', 'allowUnsafeConfigPaths'],
2929
['GIT_CONFIG_SYSTEM', '/tmp/malicious', 'allowUnsafeConfigPaths'],
3030
['GIT_CONFIG_COUNT', '1', 'allowUnsafeConfigEnvCount'],
31+
['GIT_CONFIG_PARAMETERS', "'core.pager=cat'", 'allowUnsafeConfigEnvCount'],
3132
['GIT_CONFIG', '/tmp/malicious', 'allowUnsafeConfigPaths'],
3233
['GIT_EDITOR', '/tmp/malicious', 'allowUnsafeEditor'],
3334
['GIT_SEQUENCE_EDITOR', '/tmp/malicious', 'allowUnsafeEditor'],

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ describe('VulnerabilityAnalysis', () => {
6565
['diff.command', 'malicious', 'allowUnsafeDiffExternal'],
6666
['diff.driver.command', 'malicious', 'allowUnsafeDiffExternal'],
6767
['diff.external', 'malicious', 'allowUnsafeDiffExternal'],
68+
['difftool.vimdiff.cmd', 'malicious', 'allowUnsafeDiffExternal'],
6869
['diff.textconv', 'malicious', 'allowUnsafeDiffTextConv'],
6970
['diff.driver.textconv', 'malicious', 'allowUnsafeDiffTextConv'],
7071
['filter.clean', 'malicious', 'allowUnsafeFilter'],
@@ -74,6 +75,8 @@ describe('VulnerabilityAnalysis', () => {
7475
['gpg.program', 'malicious', 'allowUnsafeGpgProgram'],
7576
['gpg.type.program', 'malicious', 'allowUnsafeGpgProgram'],
7677
['init.templateDir', '/tmp/evil', 'allowUnsafeTemplateDir'],
78+
['pager.log', 'malicious', 'allowUnsafePager'],
79+
['pager.log.color', 'malicious', 'allowUnsafePager'],
7780
['merge.driver', '/tmp/evil', 'allowUnsafeMergeDriver'],
7881
['merge.foo.driver', '/tmp/evil', 'allowUnsafeMergeDriver'],
7982
['mergetool.path', '/tmp/evil', 'allowUnsafeMergeDriver'],
@@ -84,6 +87,7 @@ describe('VulnerabilityAnalysis', () => {
8487
['remote.https://example.com.receivepack', 'malicious', 'allowUnsafePack'],
8588
['remote.uploadpack', 'malicious', 'allowUnsafePack'],
8689
['remote.https://example.com.uploadpack', 'malicious', 'allowUnsafePack'],
90+
['uploadpack.packObjectsHook', 'malicious', 'allowUnsafePack'],
8791
['sequence.editor', 'malicious', 'allowUnsafeEditor'],
8892
])('writing %s = %s to the git config', (key, value, category) => {
8993
const expected = category ? oneVulnerability(category) : noVulnerabilities();

‎yarn.lock‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# Manual changes might be lost - proceed with caution!
33

44
__metadata:
5-
version: 8
5+
version: 9
66
cacheKey: 10
77

88
"@ampproject/remapping@npm:^2.2.0":

0 commit comments

Comments
 (0)