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.

Sign up for GitHub

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, os/exec: unsanitized NUL in environment variables #56284

Closed
neild opened this issue Oct 18, 2022 · 14 comments
Closed

syscall, os/exec: unsanitized NUL in environment variables #56284

neild opened this issue Oct 18, 2022 · 14 comments
Assignees
@neild
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge release-blocker Security
Milestone
Go1.20

Comments

@neild
Copy link
Contributor

neild commented Oct 18, 2022 •
edited by mdempsky
Loading

On Windows, syscall.StartProcess and os/exec.Cmd did not properly check for invalid environment variable values. A malicious environment variable value could exploit this behavior to set a value for a different environment variable. For example, the environment variable string "A=B\x00C=D" set the variables "A=B" and "C=D".

Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this issue.

This is CVE-2022-41716 and Go issue https://go.dev/issue/56284.
@neild neild self-assigned this Oct 18, 2022
@heschi
Copy link
Contributor

heschi commented Oct 19, 2022

This issue is currently not marked as security or release-blocking. Should it block the next minor release?

@heschi
Copy link
Contributor

heschi commented Oct 19, 2022

cc @golang/security @golang/release

@neild neild modified the milestones: Go1.19.3, Go1.18.8, Go1.20 Oct 19, 2022
@neild
Copy link
Contributor Author

neild commented Oct 19, 2022

@gopherbot please open backport issues

(Sorry, I keep getting the process on this wrong. This issue is for the fix, backport issues for upcoming minor releases.)

This was referenced Oct 19, 2022
Closed
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #56327 (for 1.18), #56328 (for 1.19).

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

@seankhliao seankhliao mentioned this issue Oct 29, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/446915 mentions this issue: [release-branch.go1.18] syscall, os/exec: reject environment variables containing NULs

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/446879 mentions this issue: [release-branch.go1.19] syscall, os/exec: reject environment variables containing NULs

@mdempsky mdempsky changed the title security: fix CVE-2022-41716 syscall, os/exec: unsanitized NUL in environment variables Nov 1, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Nov 1, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/446916 mentions this issue: syscall, os/exec: reject environment variables containing NULs

gopherbot pushed a commit that referenced this issue Nov 1, 2022
@neild @mdempsky
[release-branch.go1.18] syscall, os/exec: reject environment variable…
aba57b0 
…s containing NULs

Check for and reject environment variables containing NULs.

The conventions for passing environment variables to subprocesses
cause most or all systems to interpret a NUL as a separator. The
syscall package rejects environment variables containing a NUL
on most systems, but erroneously did not do so on Windows. This
causes an environment variable such as "FOO=a\x00BAR=b" to be
interpreted as "FOO=a", "BAR=b".

Check for and reject NULs in environment variables passed to
syscall.StartProcess on Windows.

Add a redundant check to os/exec as extra insurance.

Updates #56284
Fixes #56327
Fixes CVE-2022-41716

Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
(cherry picked from commit 845accdebb2772c5344ed0c96df9910f3b02d741)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1617552
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/446915
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
gopherbot pushed a commit that referenced this issue Nov 1, 2022
@neild @mdempsky
[release-branch.go1.19] syscall, os/exec: reject environment variable…
2a7adf4 
…s containing NULs

Check for and reject environment variables containing NULs.

The conventions for passing environment variables to subprocesses
cause most or all systems to interpret a NUL as a separator. The
syscall package rejects environment variables containing a NUL
on most systems, but erroneously did not do so on Windows. This
causes an environment variable such as "FOO=a\x00BAR=b" to be
interpreted as "FOO=a", "BAR=b".

Check for and reject NULs in environment variables passed to
syscall.StartProcess on Windows.

Add a redundant check to os/exec as extra insurance.

Updates #56284
Fixes #56328
Fixes CVE-2022-41716

Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
(cherry picked from commit 845accdebb2772c5344ed0c96df9910f3b02d741)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1617553
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/446879
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@ldemailly
Copy link

can you confirm this only affects windows? so if not cross compiling or on windows the upgrade from 1.19.2 to 1.19.3 isn't critical?

@neild
Copy link
Contributor Author

neild commented Nov 1, 2022

Yes, this only affects Go programs running on Windows, which execute subprocesses, pass environment variables to those subprocesses, and use untrusted sources for the values of those environment variables.

This was referenced Nov 1, 2022
Merged
Merged
@k0s-bot k0s-bot mentioned this issue Nov 1, 2022
Merged
@apparentlymart apparentlymart mentioned this issue Nov 1, 2022
Merged
jnummelin added a commit to jnummelin/k0s that referenced this issue Nov 2, 2022
@jnummelin
Bump Go to v1.19.3
39c981c 
https://go.dev/doc/devel/release#go1.19.3
https://groups.google.com/g/golang-announce/c/mbHY1UY3BaM
golang/go#56284

Signed-off-by: Jussi Nummelin <jnummelin@mirantis.com>
jnummelin added a commit to jnummelin/k0s that referenced this issue Nov 2, 2022
@jnummelin
Bump Go to v1.19.3
18b8485 
https://go.dev/doc/devel/release#go1.19.3
https://groups.google.com/g/golang-announce/c/mbHY1UY3BaM
golang/go#56284

Signed-off-by: Jussi Nummelin <jnummelin@mirantis.com>
apparentlymart added a commit to hashicorp/terraform that referenced this issue Nov 2, 2022
@apparentlymart
Build with Go 1.19.3
241bd7c 
This includes a small selection of security-related fixes which do not
urgently impact Terraform's behavior but do close some potential avenues
for unbounded resource usage or misbehavior with malicious input:

 - golang/go#54853
 - golang/go#55949
 - golang/go#56284
@teamterraform teamterraform mentioned this issue Nov 2, 2022
Merged
@millerresearch millerresearch mentioned this issue Nov 3, 2022
Closed
@millerresearch
Copy link
Contributor

This breaks go build for Plan 9 (see #56544),

@millerresearch millerresearch reopened this Nov 3, 2022
@mdempsky
Copy link
Contributor

mdempsky commented Nov 3, 2022

@millerresearch Thanks. We'll track fixing Plan 9 in that issue.

@mdempsky mdempsky closed this as completed Nov 3, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/447715 mentions this issue: os/exec: allow NUL in environment variables on Plan 9

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@neild @romaindoumenc
syscall, os/exec: reject environment variables containing NULs
2052453 
Check for and reject environment variables containing NULs.

The conventions for passing environment variables to subprocesses
cause most or all systems to interpret a NUL as a separator. The
syscall package rejects environment variables containing a NUL
on most systems, but erroniously did not do so on Windows. This
causes an environment variable such as "FOO=a\x00BAR=b" to be
interpreted as "FOO=a", "BAR=b".

Check for and reject NULs in environment variables passed to
syscall.StartProcess on Windows.

Add a redundant check to os/exec as extra insurance.

Fixes golang#56284
Fixes CVE-2022-41716

Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/446916
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
gopherbot pushed a commit that referenced this issue Nov 4, 2022
@mdempsky
os/exec: allow NUL in environment variables on Plan 9
bc2dc23 
Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.

For #56284.
Fixes #56544.

Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/447875 mentions this issue: [release-branch.go1.18] os/exec: allow NUL in environment variables on Plan 9

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/447799 mentions this issue: [release-branch.go1.19] os/exec: allow NUL in environment variables on Plan 9

@DrJosh9000 DrJosh9000 mentioned this issue Nov 9, 2022
Merged
gopherbot pushed a commit that referenced this issue Nov 9, 2022
@mdempsky @mknyszek
[release-branch.go1.18] os/exec: allow NUL in environment variables o…
ca57180 
…n Plan 9

Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.

For #56284.
For #56544.
Fixes #56550.

Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/447875
Run-TryBot: David du Colombier <0intro@gmail.com>
gopherbot pushed a commit that referenced this issue Nov 9, 2022
@mdempsky @mknyszek
[release-branch.go1.19] os/exec: allow NUL in environment variables o…
a2335d0 
…n Plan 9

Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.

For #56284.
For #56544.
Fixes #56551.

Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/447799
Run-TryBot: David du Colombier <0intro@gmail.com>
andrew-d pushed a commit to tailscale/go that referenced this issue Dec 7, 2022
@neild @andrew-d
[release-branch.go1.19] syscall, os/exec: reject environment variable…
54f28f9 
…s containing NULs

Check for and reject environment variables containing NULs.

The conventions for passing environment variables to subprocesses
cause most or all systems to interpret a NUL as a separator. The
syscall package rejects environment variables containing a NUL
on most systems, but erroneously did not do so on Windows. This
causes an environment variable such as "FOO=a\x00BAR=b" to be
interpreted as "FOO=a", "BAR=b".

Check for and reject NULs in environment variables passed to
syscall.StartProcess on Windows.

Add a redundant check to os/exec as extra insurance.

Updates golang#56284
Fixes golang#56328
Fixes CVE-2022-41716

Change-Id: I2950e2b0cb14ebd26e5629be1521858f66a7d4ae
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1609434
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
(cherry picked from commit 845accdebb2772c5344ed0c96df9910f3b02d741)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1617553
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/446879
Reviewed-by: Tatiana Bradley <tatiana@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
andrew-d pushed a commit to tailscale/go that referenced this issue Dec 7, 2022
@mdempsky @andrew-d
[release-branch.go1.19] os/exec: allow NUL in environment variables o…
39fbccc 
…n Plan 9

Plan 9 uses NUL as os.PathListSeparator, so it's almost always going
to appear in the environment variable list. Exempt GOOS=plan9 from the
check for NUL in environment variables.

For golang#56284.
For golang#56544.
Fixes golang#56551.

Change-Id: I23df233cdf20c0a9a606fd9253e15a9b5482575a
Reviewed-on: https://go-review.googlesource.com/c/go/+/447715
Reviewed-by: David du Colombier <0intro@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/447799
Run-TryBot: David du Colombier <0intro@gmail.com>
@matloob matloob assigned matloob and neild and unassigned neild and matloob May 11, 2023
@golang golang locked and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@neild neild

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge release-blocker Security
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
7 participants
@mdempsky @neild @ldemailly @gopherbot @matloob @millerresearch @heschi