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

Jump to bottom

syscall: don’t close fd 0 on ForkExec error #50057

Closed
FiloSottile opened this issue Dec 9, 2021 · 10 comments
Closed

syscall: don’t close fd 0 on ForkExec error #50057

FiloSottile opened this issue Dec 9, 2021 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone
Go1.18

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Dec 9, 2021 •
edited

When a Go program running on a Unix system is out of file descriptors and calls syscall.ForkExec (including indirectly by using the os/exec package), syscall.ForkExec can close file descriptor 0 as it fails. If this happens (or can be provoked) repeatedly, it can result in misdirected I/O such as writing network traffic intended for one connection to a different connection, or content intended for one file to a different one.

For users who cannot immediately update to the new release, the bug can be mitigated by raising the per-process file descriptor limit.

Thank you to Tomasz Maczukin and Kamil Trzciński of GitLab for reporting this issue.

This is CVE-2021-44717 and is fixed in Go 1.17.5 and Go 1.16.12.
@golang golang locked and limited conversation to collaborators Dec 9, 2021
@gopherbot
Copy link

Change https://golang.org/cl/370515 mentions this issue: [release-branch.go1.16] syscall: avoid writing to p when Pipe(p) fails

@gopherbot
Copy link

Change https://golang.org/cl/370514 mentions this issue: [release-branch.go1.16] syscall: fix ForkLock spurious close(0) on pipe failure

@gopherbot
Copy link

Change https://golang.org/cl/370535 mentions this issue: [release-branch.go1.17] syscall: avoid writing to p when Pipe(p) fails

@gopherbot
Copy link

Change https://golang.org/cl/370534 mentions this issue: [release-branch.go1.17] syscall: fix ForkLock spurious close(0) on pipe failure

@gopherbot
Copy link

Change https://golang.org/cl/370576 mentions this issue: syscall: fix ForkLock spurious close(0) on pipe failure

@gopherbot
Copy link

Change https://golang.org/cl/370577 mentions this issue: syscall: avoid writing to p when Pipe(p) fails

@gopherbot
Copy link

Change https://golang.org/cl/370614 mentions this issue: unix, plan9: avoid writing to p when Pipe(p) fails

@FiloSottile FiloSottile added NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security labels Dec 9, 2021
@FiloSottile FiloSottile added this to the Go1.18 milestone Dec 9, 2021
@FiloSottile FiloSottile changed the title placeholder syscall: don’t close fd 0 on ForkExec error Dec 9, 2021
@golang golang unlocked this conversation Dec 9, 2021
@FiloSottile
Copy link
Contributor Author

@gopherbot please open backport issues, this is CVE-2021-44717.

This was referenced Dec 9, 2021
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #50066 (for 1.16), #50067 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

pull bot pushed a commit to Pandinosaurus/go that referenced this issue Dec 9, 2021
@rsc @FiloSottile
syscall: avoid writing to p when Pipe(p) fails
474ebb9 
Generally speaking Go functions make no guarantees
about what has happened to result parameters on error,
and Pipe is no exception: callers should avoid looking at
p if Pipe returns an error.

However, we had a bug in which ForkExec was using the
content of p after a failed Pipe, and others may too.
As a robustness fix, make Pipe avoid writing to p on failure.

Updates golang#50057

Change-Id: Ie8955025dbd20702fabadc9bbe1d1a5ac0f36305
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291271
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370577
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
gopherbot pushed a commit to golang/sys that referenced this issue Dec 9, 2021
@rsc
unix, plan9: avoid writing to p when Pipe(p) fails
798191b 
Generally speaking Go functions make no guarantees
about what has happened to result parameters on error,
and Pipe is no exception: callers should avoid looking at
p if Pipe returns an error.

However, we had a bug in which ForkExec was using the
content of p after a failed Pipe, and others may too.
As a robustness fix, make Pipe avoid writing to p on failure.

windows.Pipe already avoided writing to p on failure.

For golang/go#50057.

Change-Id: I93ed06b06a9981793c119c1d7df689fbe79b4116
Reviewed-on: https://go-review.googlesource.com/c/sys/+/370614
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
josharian pushed a commit to tailscale/go that referenced this issue Dec 9, 2021
@rsc @FiloSottile
[release-branch.go1.17] syscall: fix ForkLock spurious close(0) on pi…
e46abcb 
…pe failure

Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Updates golang#50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370534
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
josharian pushed a commit to tailscale/go that referenced this issue Dec 9, 2021
@rsc @FiloSottile
[release-branch.go1.17] syscall: avoid writing to p when Pipe(p) fails
65633fe 
Generally speaking Go functions make no guarantees
about what has happened to result parameters on error,
and Pipe is no exception: callers should avoid looking at
p if Pipe returns an error.

However, we had a bug in which ForkExec was using the
content of p after a failed Pipe, and others may too.
As a robustness fix, make Pipe avoid writing to p on failure.

Updates golang#50057

Change-Id: Ie8955025dbd20702fabadc9bbe1d1a5ac0f36305
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291271
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370535
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
gopherbot pushed a commit that referenced this issue Dec 9, 2021
@rsc @FiloSottile
[release-branch.go1.16] syscall: fix ForkLock spurious close(0) on pi…
44a3fb4 
…pe failure

Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Updates #50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370514
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
gopherbot pushed a commit that referenced this issue Dec 9, 2021
@rsc @FiloSottile
[release-branch.go1.16] syscall: avoid writing to p when Pipe(p) fails
9995027 
Generally speaking Go functions make no guarantees
about what has happened to result parameters on error,
and Pipe is no exception: callers should avoid looking at
p if Pipe returns an error.

However, we had a bug in which ForkExec was using the
content of p after a failed Pipe, and others may too.
As a robustness fix, make Pipe avoid writing to p on failure.

Updates #50057

Change-Id: Ie8955025dbd20702fabadc9bbe1d1a5ac0f36305
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291271
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370515
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/370795 mentions this issue: plan9, unix: avoid writing to p when Pipe(p) fails

@bflad bflad mentioned this issue Dec 14, 2021
Closed
danbudris pushed a commit to danbudris/go that referenced this issue Sep 14, 2022
@rsc @danbudris
syscall: fix ForkLock spurious close(0) on pipe failure
fec4bf7 
Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Fixes golang#50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370576
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
rcrozean pushed a commit to rcrozean/go that referenced this issue Oct 5, 2022
@rsc @rcrozean
syscall: fix ForkLock spurious close(0) on pipe failure
8c7d670 
# AWS EKS
Backported To: go-1.15.15-eks
Backported On: Thu, 22 Sept 2022
Backported By: budris@amazon.com
Backported From: release-branch.go1.18
Upstream Source Commit: golang@a76511f
EKS Patch Source Commit: danbudris@fec4bf7

# Original Information

Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Fixes golang#50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370576
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
rcrozean pushed a commit to rcrozean/go that referenced this issue Oct 12, 2022
@rsc @rcrozean
syscall: fix ForkLock spurious close(0) on pipe failure
6439d1f 
# AWS EKS
Backported To: go-1.15.15-eks
Backported On: Thu, 22 Sept 2022
Backported By: budris@amazon.com
Backported From: release-branch.go1.18
Upstream Source Commit: golang@a76511f
EKS Patch Source Commit: danbudris@fec4bf7

# Original Information

Pipe (and therefore forkLockPipe) does not make any guarantees
about the state of p after a failed Pipe(p). Avoid that assumption
and the too-clever goto, so that we don't accidentally Close a real fd
if the failed pipe leaves p[0] or p[1] set >= 0.

Fixes golang#50057
Fixes CVE-2021-44717

Change-Id: Iff8e19a6efbba0c73cc8b13ecfae381c87600bb4
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1291270
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/370576
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Alex Rakoczy <alex@golang.org>
@golang golang locked and limited conversation to collaborators Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
2 participants
@FiloSottile @gopherbot