New issue

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

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

Already on GitHub? Sign in to your account

cmd/go: arbitrary code execution during “go get” or “go get -d” [Go 1.8] #22125

Closed
broady opened this Issue Oct 4, 2017 · 7 comments

Comments

Projects
None yet
6 participants
@broady
Member

broady commented Oct 4, 2017

Using custom domains, it is possible to arrange things so that example.com/pkg1 points to a Subversion repository but example.com/pkg1/pkg2 points to a Git repository. If the Subversion repository includes a Git checkout in its pkg2 directory and some other work is done to ensure the proper ordering of operations, “go get” can be tricked into reusing this Git checkout for the fetch of code from pkg2. If the Subversion repository’s Git checkout has a malicious code in .git/hooks/, it will execute on the system running “go get.”

The fix is to detect version control checkouts nested inside other version control checkouts and refuse to execute version control operations in those checkouts. It would be nice if Git had a --no-hooks option, and then we could use that as an additional backup step, but it does not.

Fixed in Go 1.8.4 by CL 68190 (a4544a0).
Fixed in Go 1.9.1 by CL 68022 (a39bcec).

Thanks to Simon Rawet for reporting this problem.

Update: This has been assigned CVE-2017-15041.

@broady broady added this to the Go1.8.4 milestone Oct 4, 2017

@rsc rsc changed the title from placeholder for security issue to cmd/go: arbitrary code execution during “go get” or “go get -d” [Go 1.8] Oct 4, 2017

@rsc rsc closed this Oct 4, 2017

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Oct 5, 2017

Member

Doesn't this need to be applied to golang.org/x/tools/go/vcs package as well (due to #11490 not being resolved)? Is there an existing issue tracking that?

Edit: Done in CL 68352.

Member

dmitshur commented Oct 5, 2017

Doesn't this need to be applied to golang.org/x/tools/go/vcs package as well (due to #11490 not being resolved)? Is there an existing issue tracking that?

Edit: Done in CL 68352.

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot Oct 5, 2017

Change https://golang.org/cl/68352 mentions this issue: go/vcs: reject update of VCS inside VCS

gopherbot commented Oct 5, 2017

Change https://golang.org/cl/68352 mentions this issue: go/vcs: reject update of VCS inside VCS

gopherbot pushed a commit to golang/tools that referenced this issue Oct 5, 2017

go/vcs: reject update of VCS inside VCS
Manually apply same change as CL 68110 did for cmd/go/internal/get,
but for golang.org/x/tools/go/vcs, to help keep them in sync.

Updates golang/go#22125.
Helps golang/go#11490.

Change-Id: I255f7a494d9572389fc8dc8ce96891b6fcc214a0
Reviewed-on: https://go-review.googlesource.com/68352
Reviewed-by: Russ Cox <rsc@golang.org>
@artursapek

This comment has been minimized.

Show comment
Hide comment
@artursapek

artursapek Oct 5, 2017

This change seems to break go get on a nested bzr package called labix.org/v2/mgo.

Given command go get labix.org/v2/mgo/bson, we get this output:

package labix.org/v2/mgo/bson: directory "/home/emile/go/src/labix.org/v2/mgo" uses bzr, but parent "/home/emile/go/src/labix.org/v2" uses bzr

Same goes for this mirror: go get gopkg.in/mgo.v2/bson

Was the intent here to disable all nested VCS except for git?

artursapek commented Oct 5, 2017

This change seems to break go get on a nested bzr package called labix.org/v2/mgo.

Given command go get labix.org/v2/mgo/bson, we get this output:

package labix.org/v2/mgo/bson: directory "/home/emile/go/src/labix.org/v2/mgo" uses bzr, but parent "/home/emile/go/src/labix.org/v2" uses bzr

Same goes for this mirror: go get gopkg.in/mgo.v2/bson

Was the intent here to disable all nested VCS except for git?

@KilledKenny

This comment has been minimized.

Show comment
Hide comment
@KilledKenny

KilledKenny Oct 5, 2017

Contributor

@artursapek That bad we didn't know that nested bzr was allowed while developing this fix.
The goal was to prevent malicious uses of mixing VCS.
The reason why git was allowed inside git was because that was know safe behavior.

Contributor

KilledKenny commented Oct 5, 2017

@artursapek That bad we didn't know that nested bzr was allowed while developing this fix.
The goal was to prevent malicious uses of mixing VCS.
The reason why git was allowed inside git was because that was know safe behavior.

@artursapek

This comment has been minimized.

Show comment
Hide comment
@artursapek

artursapek Oct 5, 2017

@KilledKenny ok, might be worth adding an exception for bzr as well if it's deemed safe. I know this mgo package is pretty popular and these changes broke installation for it. That said I do wish they would just use git, I mean come on : )

artursapek commented Oct 5, 2017

@KilledKenny ok, might be worth adding an exception for bzr as well if it's deemed safe. I know this mgo package is pretty popular and these changes broke installation for it. That said I do wish they would just use git, I mean come on : )

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Oct 5, 2017

Contributor

Thanks @artursapek; filed #22157.

Contributor

rsc commented Oct 5, 2017

Thanks @artursapek; filed #22157.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Oct 5, 2017

Contributor

Update: This has been assigned CVE-2017-15041. (Also added to top comment.)

Contributor

rsc commented Oct 5, 2017

Update: This has been assigned CVE-2017-15041. (Also added to top comment.)

mdeuser added a commit to mdeuser/openwhisk that referenced this issue Oct 25, 2017

Bump go cli to 1.8.4
- Fix for security issue golang/go#22125

csantanapr added a commit to apache/incubator-openwhisk that referenced this issue Oct 31, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this issue Nov 13, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this issue Nov 13, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this issue Nov 14, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this issue Nov 15, 2017

houshengbo pushed a commit to houshengbo/openwhisk that referenced this issue Nov 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment