Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

settings: require explicit user action before using workspace/folder settings #1094

Closed
hyangah opened this issue Jan 14, 2021 · 6 comments
Closed

settings: require explicit user action before using workspace/folder settings #1094

hyangah opened this issue Jan 14, 2021 · 6 comments
Labels
FrozenDueToAge
Milestone
v0.21.0

Comments

@hyangah
Copy link
Contributor

hyangah commented Jan 14, 2021 •
edited
Loading

Many of VS Code Go settings are defined as resource or machine-overridable scope configuration, which means they can be specified in the workspace or folder levels. Some of the settings, for example, "go.goroot", "go.alternateTools", "go.gopath", "go.toolsGopath", "go.inferGopath", ... can affect the location of tools and binaries the extension runs. While this is convenient (*), it also can be exploited to run any binaries on users' machines once if the users are tricked to check out a random repository with workspace settings and open it with vscode.

This vulnerability issue around workspace/folder level settings is not new:

While hoping to have a unified approach for this class of issues eventually, we want to address this issue from our extension now.

When opening workspace/folder-level configuration for those settings, the extension will ignore them by default and warn users. If users trust the workspace, they can explicitly tell the extension by running a command.

The affected settings (**) are:

  • go.goroot
  • go.gopath
  • go.toolsGopath
  • go.inferGopath
  • go.alternateTools
  • go.toolsEnvVars

cc @FiloSottile @rsc @suzmue @pjweinb @stamblerre

*: when we previously tried to convert some of those settings to machine scope ones for different reasons, the change broke many users' workflow and had to be reverted.
**: should we include all the settings by default to be safe? (there could be more settings that can be potentially exploitable...)
We will consider adding restrictions to additional settings like go.testOnSave.
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/283256 mentions this issue: package.json: update settings description to mark dangerous settings

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/283253 mentions this issue: src/config: add Configuration class to encapsulate the extension settings

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/283257 mentions this issue: src/util: resolveToolsGopath ignores toolsGopath from untrusted workspace

@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/283255 mentions this issue: src/config: move getGoConfig/getGoplsConfig to config

@hyangah hyangah added this to the v0.21.0 milestone Jan 14, 2021
gopherbot pushed a commit that referenced this issue Jan 14, 2021
@hyangah
src/config: add Configuration class to encapsulate the extension sett…
1ac4671 
…ings

Configuration's get returns the 'go' section of the vscode settings.
This wraps the vscode.WorkspaceConfiguration object that would be returned
by vscode.workspace.getConfiguration, but this prevents it from returning
values from the workspace/workspaceFolder level settings if the queried
key is security-sensitive - e.g. use of workspace level settings from
the untrusted repository may result in arbitrary binary execution.

This CL does not use the new Configuration from the extension yet.

For #1094

Change-Id: Ia75389032dfaec300506b26ab839b173ff9e5557
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/283253
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopherbot pushed a commit that referenced this issue Jan 15, 2021
@hyangah
src/config: move getGoConfig/getGoplsConfig to config
bc8b63e 
And let them use the new Configuration class

For #1094

Change-Id: I96d0cc32b6499e59d1062c2b04031bc675703a24
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/283255
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopherbot pushed a commit that referenced this issue Jan 15, 2021
@hyangah
package.json: update settings description to mark dangerous settings
e1a6cb4 
Also add 'go.inferGopath' to the dangerous setting list.

For #1094

Change-Id: If5c936d6a875b92aef6d8368e14d02c66bbae7ce
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/283256
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopherbot pushed a commit that referenced this issue Jan 15, 2021
@hyangah
src/util: resolveToolsGopath ignores toolsGopath from untrusted works…
3e3f506 
…pace

And initialize the default config object when the module loads,
and reuse it.

For #1094

Change-Id: Ia80d703487f6a717dc7aed6d52baeb73c4b49707
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/283257
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@hyangah
Copy link
Contributor Author

hyangah commented Jan 20, 2021

v0.21.0 is out.

@hyangah hyangah closed this as completed Jan 20, 2021
@gopherbot
Copy link
Collaborator

Change https://golang.org/cl/300289 mentions this issue: src/config: add toolsEnvVars to the config list to watch

gopherbot pushed a commit that referenced this issue Mar 10, 2021
@hyangah
src/config: add toolsEnvVars to the config list to watch
b085ad4 
For #1094

Change-Id: Ibe4c405271404ff08200ab6f5d5afcc2a4e162e6
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/300289
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
@golang golang locked and limited conversation to collaborators Mar 10, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
v0.21.0
Development

No branches or pull requests
2 participants
@hyangah @gopherbot