Skip to content
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

os: windows processes started with foreign token inherit the wrong environment [CVE-2019-11888] [1.12 backport] #32081

Closed
zx2c4 opened this issue May 16, 2019 · 8 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@zx2c4
Copy link
Contributor

zx2c4 commented May 16, 2019

We should probably backport #32000. IIRC, I'm supposed to open an issue for it.

@gopherbot gopherbot added this to the Go1.12.6 milestone May 16, 2019
@ALTree
Copy link
Member

ALTree commented May 16, 2019

Standard way to to this is to write:

@gopherbot Please open a backport to 1.12.

in the main issue.

@gopherbot

This comment has been minimized.

@ALTree
Copy link
Member

ALTree commented May 16, 2019

Ops, it did it even if it was in a quote block... fixing this.

@zx2c4
Copy link
Contributor Author

zx2c4 commented May 16, 2019

Closed this one, wrote the magic words in the other one.

@zx2c4 zx2c4 reopened this May 16, 2019
@zx2c4
Copy link
Contributor Author

zx2c4 commented May 16, 2019

Welp, I guess we've confused gopherbot now. Probably add "CherryPickCandidate" or something manually?

@gopherbot
Copy link

Change https://golang.org/cl/177538 mentions this issue: [release-branch.go1.12] os: pass correct environment when creating Windows processes

@ALTree ALTree added the CherryPickCandidate Used during the release process for point releases label May 16, 2019
@ALTree
Copy link
Member

ALTree commented May 16, 2019

Welp, I guess we've confused gopherbot now. Probably add "CherryPickCandidate" or something manually?

Done. I hope we didn't confuse him too much. Well, the backport issue here is correctly labelled and it's in the right milestone, so it should be fine. Let's keep an eye on this issue in any case.

gopherbot pushed a commit that referenced this issue May 17, 2019
…ndows processes

This is CVE-2019-11888.

Previously, passing a nil environment but a non-nil token would result
in the new potentially unprivileged process inheriting the parent
potentially privileged environment, or would result in the new
potentially privileged process inheriting the parent potentially
unprivileged environment. Either way, it's bad. In the former case, it's
an infoleak. In the latter case, it's a possible EoP, since things like
PATH could be overwritten.

Not specifying an environment currently means, "use the existing
environment". This commit amends the behavior to be, "use the existing
environment of the token the process is being created for." The behavior
therefore stays the same when creating processes without specifying a
token. And it does the correct thing when creating processes when
specifying a token.

Updates #32000
Fixes #32081

Change-Id: Ib4a90cfffb6ba866c855f66f1313372fdd34ce41
Reviewed-on: https://go-review.googlesource.com/c/go/+/177538
Run-TryBot: Jason Donenfeld <Jason@zx2c4.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@FiloSottile
Copy link
Contributor

Looks like this was merged and it did not get closed.

@FiloSottile FiloSottile added the CherryPickApproved Used during the release process for point releases label May 17, 2019
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label May 17, 2019
@golang golang locked and limited conversation to collaborators May 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

4 participants