From 5ef5837c581469dfd72aea765b10e0a6241f7690 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 9 Jul 2021 15:43:15 -0400 Subject: [PATCH] =?UTF-8?q?Improves=C2=A0Git=C2=A0security=C2=A0with=C2=A0?= =?UTF-8?q?untrusted=C2=A0workspaces?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- extensions/git/src/git.ts | 44 +++++++++++++++++++----------------- extensions/git/src/main.ts | 26 +++++++++++++++++++-- extensions/git/src/model.ts | 45 +++++++++++++++++++++++++------------ 3 files changed, 78 insertions(+), 37 deletions(-) diff --git a/extensions/git/src/git.ts b/extensions/git/src/git.ts index ae318036756..fab81e408ae 100644 --- a/extensions/git/src/git.ts +++ b/extensions/git/src/git.ts @@ -63,9 +63,11 @@ function parseVersion(raw: string): string { return raw.replace(/^git version /, ''); } -function findSpecificGit(path: string, onLookup: (path: string) => void): Promise { +function findSpecificGit(path: string, onValidate: (path: string) => boolean): Promise { return new Promise((c, e) => { - onLookup(path); + if (!onValidate(path)) { + return e('git not found'); + } const buffers: Buffer[] = []; const child = cp.spawn(path, ['--version']); @@ -75,7 +77,7 @@ function findSpecificGit(path: string, onLookup: (path: string) => void): Promis }); } -function findGitDarwin(onLookup: (path: string) => void): Promise { +function findGitDarwin(onValidate: (path: string) => boolean): Promise { return new Promise((c, e) => { cp.exec('which git', (err, gitPathBuffer) => { if (err) { @@ -85,7 +87,9 @@ function findGitDarwin(onLookup: (path: string) => void): Promise { const path = gitPathBuffer.toString().replace(/^\s+|\s+$/g, ''); function getVersion(path: string) { - onLookup(path); + if (!onValidate(path)) { + return e('git not found'); + } // make sure git executes cp.exec('git --version', (err, stdout) => { @@ -117,33 +121,31 @@ function findGitDarwin(onLookup: (path: string) => void): Promise { }); } -function findSystemGitWin32(base: string, onLookup: (path: string) => void): Promise { +function findSystemGitWin32(base: string, onValidate: (path: string) => boolean): Promise { if (!base) { return Promise.reject('Not found'); } - return findSpecificGit(path.join(base, 'Git', 'cmd', 'git.exe'), onLookup); + return findSpecificGit(path.join(base, 'Git', 'cmd', 'git.exe'), onValidate); } -function findGitWin32InPath(onLookup: (path: string) => void): Promise { +function findGitWin32InPath(onValidate: (path: string) => boolean): Promise { const whichPromise = new Promise((c, e) => which('git.exe', (err, path) => err ? e(err) : c(path))); - return whichPromise.then(path => findSpecificGit(path, onLookup)); + return whichPromise.then(path => findSpecificGit(path, onValidate)); } -function findGitWin32(onLookup: (path: string) => void): Promise { - return findSystemGitWin32(process.env['ProgramW6432'] as string, onLookup) - .then(undefined, () => findSystemGitWin32(process.env['ProgramFiles(x86)'] as string, onLookup)) - .then(undefined, () => findSystemGitWin32(process.env['ProgramFiles'] as string, onLookup)) - .then(undefined, () => findSystemGitWin32(path.join(process.env['LocalAppData'] as string, 'Programs'), onLookup)) - .then(undefined, () => findGitWin32InPath(onLookup)); +function findGitWin32(onValidate: (path: string) => boolean): Promise { + return findSystemGitWin32(process.env['ProgramW6432'] as string, onValidate) + .then(undefined, () => findSystemGitWin32(process.env['ProgramFiles(x86)'] as string, onValidate)) + .then(undefined, () => findSystemGitWin32(process.env['ProgramFiles'] as string, onValidate)) + .then(undefined, () => findSystemGitWin32(path.join(process.env['LocalAppData'] as string, 'Programs'), onValidate)) + .then(undefined, () => findGitWin32InPath(onValidate)); } -export async function findGit(hint: string | string[] | undefined, onLookup: (path: string) => void): Promise { - const hints = Array.isArray(hint) ? hint : hint ? [hint] : []; - +export async function findGit(hints: string[], onValidate: (path: string) => boolean): Promise { for (const hint of hints) { try { - return await findSpecificGit(hint, onLookup); + return await findSpecificGit(hint, onValidate); } catch { // noop } @@ -151,9 +153,9 @@ export async function findGit(hint: string | string[] | undefined, onLookup: (pa try { switch (process.platform) { - case 'darwin': return await findGitDarwin(onLookup); - case 'win32': return await findGitWin32(onLookup); - default: return await findSpecificGit('git', onLookup); + case 'darwin': return await findGitDarwin(onValidate); + case 'win32': return await findGitWin32(onValidate); + default: return await findSpecificGit('git', onValidate); } } catch { // noop diff --git a/extensions/git/src/main.ts b/extensions/git/src/main.ts index f391996c968..0aa3193e394 100644 --- a/extensions/git/src/main.ts +++ b/extensions/git/src/main.ts @@ -34,8 +34,30 @@ export async function deactivate(): Promise { } async function createModel(context: ExtensionContext, outputChannel: OutputChannel, telemetryReporter: TelemetryReporter, disposables: Disposable[]): Promise { - const pathHint = workspace.getConfiguration('git').get('path'); - const info = await findGit(pathHint, path => outputChannel.appendLine(localize('looking', "Looking for git in: {0}", path))); + const pathValue = workspace.getConfiguration('git').get('path'); + let pathHints = Array.isArray(pathValue) ? pathValue : pathValue ? [pathValue] : []; + + const { isTrusted, workspaceFolders = [] } = workspace; + const excludes = isTrusted ? [] : workspaceFolders.map(f => path.normalize(f.uri.fsPath).replace(/[\r\n]+$/, '')); + + if (!isTrusted && pathHints.length !== 0) { + // Filter out any non-absolute paths + pathHints = pathHints.filter(p => path.isAbsolute(p)); + } + + const info = await findGit(pathHints, gitPath => { + outputChannel.appendLine(localize('validating', "Validating found git in: {0}", gitPath)); + if (excludes.length === 0) { + return true; + } + + const normalized = path.normalize(gitPath).replace(/[\r\n]+$/, ''); + const skip = excludes.some(e => normalized.startsWith(e)); + if (skip) { + outputChannel.appendLine(localize('skipped', "Skipped found git in: {0}", gitPath)); + } + return !skip; + }); const askpass = await Askpass.create(outputChannel, context.storagePath); disposables.push(askpass); diff --git a/extensions/git/src/model.ts b/extensions/git/src/model.ts index 1cd80cf6c49..9a2f9197378 100644 --- a/extensions/git/src/model.ts +++ b/extensions/git/src/model.ts @@ -147,23 +147,23 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe await Promise.all((workspace.workspaceFolders || []).map(async folder => { const root = folder.uri.fsPath; const children = await new Promise((c, e) => fs.readdir(root, (err, r) => err ? e(err) : c(r))); - const promises = children - .filter(child => child !== '.git') - .map(child => this.openRepository(path.join(root, child))); + const subfolders = new Set(children.filter(child => child !== '.git').map(child => path.join(root, child))); - const folderConfig = workspace.getConfiguration('git', folder.uri); - const paths = folderConfig.get('scanRepositories') || []; + const scanPaths = (workspace.isTrusted ? workspace.getConfiguration('git', folder.uri) : config).get('scanRepositories') || []; + for (const scanPath of scanPaths) { + if (scanPath !== '.git') { + continue; + } - for (const possibleRepositoryPath of paths) { - if (path.isAbsolute(possibleRepositoryPath)) { + if (path.isAbsolute(scanPath)) { console.warn(localize('not supported', "Absolute paths not supported in 'git.scanRepositories' setting.")); continue; } - promises.push(this.openRepository(path.join(root, possibleRepositoryPath))); + subfolders.add(path.join(root, scanPath)); } - await Promise.all(promises); + await Promise.all([...subfolders].map(f => this.openRepository(f))); })); } @@ -226,6 +226,10 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe } private async onDidChangeVisibleTextEditors(editors: readonly TextEditor[]): Promise { + if (!workspace.isTrusted) { + return; + } + const config = workspace.getConfiguration('git'); const autoRepositoryDetection = config.get('autoRepositoryDetection'); @@ -251,20 +255,33 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe } @sequentialize - async openRepository(path: string): Promise { - if (this.getRepository(path)) { + async openRepository(repoPath: string): Promise { + if (this.getRepository(repoPath)) { return; } - const config = workspace.getConfiguration('git', Uri.file(path)); + const config = workspace.getConfiguration('git', Uri.file(repoPath)); const enabled = config.get('enabled') === true; if (!enabled) { return; } + if (!workspace.isTrusted) { + // Check if the folder is a bare repo: if it has a file named HEAD && `rev-parse --show -cdup` is empty + try { + fs.accessSync(path.join(repoPath, 'HEAD'), fs.constants.F_OK); + const result = await this.git.exec(repoPath, ['-C', repoPath, 'rev-parse', '--show-cdup'], { log: false }); + if (result.stderr.trim() === '' && result.stdout.trim() === '') { + return; + } + } catch { + // If this throw, we should be good to open the repo (e.g. HEAD doesn't exist) + } + } + try { - const rawRoot = await this.git.getRepositoryRoot(path); + const rawRoot = await this.git.getRepositoryRoot(repoPath); // This can happen whenever `path` has the wrong case sensitivity in // case insensitive file systems @@ -286,7 +303,7 @@ export class Model implements IRemoteSourceProviderRegistry, IPushErrorHandlerRe await repository.status(); } catch (ex) { // noop - this.outputChannel.appendLine(`Opening repository for path='${path}' failed; ex=${ex}`); + this.outputChannel.appendLine(`Opening repository for path='${repoPath}' failed; ex=${ex}`); } } -- GitLab