Skip to content

Removed Snap related code from updateService.linux.ts - #69979

Merged
João Moreno (joaomoreno) merged 1 commit into
microsoft:masterfrom
Kedstar99:UpdateServiceBranch
Mar 8, 2019
Merged

João Moreno (joaomoreno) merged 1 commit into
microsoft:masterfrom
Kedstar99:UpdateServiceBranch

Conversation

@Kedstar99

Copy link
Copy Markdown
Contributor

This patch does two things. The first removes the snap update code from updateService.linux.ts.
This is because the code always creates a SnapUpdateService if the environment variables are set. This occurs in vscode/src/vs/code/electron-main/app.ts.

The second change this patch introduces is the removal of spawning processes based on $SNAP_NAME. This is because an attacker could potentially leverage the environment variables SNAP, SNAP_REVISION and SNAP_NAME to enable execution through doQuitAndInstall.

I have instead replaced $SNAP_NAME with path.basename(process.argv[0]). This should be equivalent and should just return the name of the executing process. This will preserve the original behaviour whilst remaining compatible with all the various SNAP update mechanisms. This provides better guarantees of executing vscode than some other arbitrary program.

Fixed updateService.snap to not execute process based on environment variable.
@joaomoreno João Moreno (joaomoreno) added linux Issues with VS Code on Linux snap Issues related to the snap package labels Mar 8, 2019
@joaomoreno João Moreno (joaomoreno) added this to the March 2019 milestone Mar 8, 2019
@joaomoreno

Copy link
Copy Markdown
Contributor

Thanks for the cleanup and the fix!

@joaomoreno
João Moreno (joaomoreno) merged commit 663eca8 into microsoft:master Mar 8, 2019
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

linux Issues with VS Code on Linux snap Issues related to the snap package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants