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

cmd/go: cgo code injection [CVE-2023-29402] #60167

Closed
rolandshoemaker opened this issue May 12, 2023 · 24 comments
Closed

cmd/go: cgo code injection [CVE-2023-29402] #60167

rolandshoemaker opened this issue May 12, 2023 · 24 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Milestone
Go1.21

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented May 12, 2023 •
edited by dr2chase

The go command may generate unexpected code at build time when using cgo. This
may result in unexpected behavior when running a go program which uses cgo.

This may occur when running an untrusted module which contains directories with
newline characters in their names. Modules which are retrieved using the go command,
i.e. via "go get", are not affected (modules retrieved using GOPATH-mode, i.e.
GO111MODULE=off, may be affected).

Thanks to Juho Nurminen of Mattermost for reporting this issue.

This is a PRIVATE issue for CVE-2023-29402, tracked in http://b/280799945

/cc @golang/security and @golang/release
@dmitshur dmitshur added this to the Go1.21 milestone May 12, 2023
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 12, 2023
@rolandshoemaker rolandshoemaker added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels May 19, 2023
@rolandshoemaker
Copy link
Member Author

@gopherbot please open backport issues.

This was referenced May 30, 2023
Closed
Closed
@gopherbot
Copy link

Backport issue(s) opened: #60515 (for 1.19), #60516 (for 1.20).

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

@gopherbot
Copy link

Change https://go.dev/cl/501218 mentions this issue: [release-branch.go1.19] cmd/go: disallow package directories containing newlines

@gopherbot
Copy link

Change https://go.dev/cl/501222 mentions this issue: [release-branch.go1.20] cmd/go: disallow package directories containing newlines

@gopherbot
Copy link

Change https://go.dev/cl/501226 mentions this issue: cmd/go: disallow package directories containing newlines

gopherbot pushed a commit that referenced this issue Jun 6, 2023
@gopherbot
[release-branch.go1.20] cmd/go: disallow package directories containi…
c0ed873 
…ng newlines

Directory or file paths containing newlines may cause tools (such as
cmd/cgo) that emit "//line" or "#line" -directives to write part of
the path into non-comment lines in generated source code. If those
lines contain valid Go code, it may be injected into the resulting
binary.

(Note that Go import paths and file paths within module zip files
already could not contain newlines.)

Thanks to Juho Nurminen of Mattermost for reporting this issue.

Updates #60167.
Fixes #60516.
Fixes CVE-2023-29402.

Change-Id: Ic3c7d8d1f6460993bd93a27035d61bff7dd68832
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1882606
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 41f9046495564fc728d6f98384ab7276450ac7e2)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902230
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904347
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501222
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jun 6, 2023
@gopherbot
[release-branch.go1.19] cmd/go: disallow package directories containi…
c160b49 
…ng newlines

Directory or file paths containing newlines may cause tools (such as
cmd/cgo) that emit "//line" or "#line" -directives to write part of
the path into non-comment lines in generated source code. If those
lines contain valid Go code, it may be injected into the resulting
binary.

(Note that Go import paths and file paths within module zip files
already could not contain newlines.)

Thanks to Juho Nurminen of Mattermost for reporting this issue.

Updates #60167.
Fixes #60515.
Fixes CVE-2023-29402.

Change-Id: If55d0400c02beb7a5da5eceac60f1abeac99f064
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1882606
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Russ Cox <rsc@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit 41f9046495564fc728d6f98384ab7276450ac7e2)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1902229
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1904343
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/501218
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@dr2chase dr2chase changed the title security: fix CVE-2023-29402 cmd/go: cgo code injection [CVE-2023-29402] Jun 6, 2023
@gopherbot
Copy link

Change https://go.dev/cl/501578 mentions this issue: go/printer: error out of Fprint when it would write a '//line' directive with a multiline file path

@gopherbot
Copy link

Change https://go.dev/cl/501576 mentions this issue: cmd/cgo: error out if the source path used in line directives would contain a newline

@gopherbot
Copy link

Change https://go.dev/cl/501575 mentions this issue: cmd/go: fix TestScript/build_cwd_newline with CGO_ENABLED=0

@gopherbot
Copy link

Change https://go.dev/cl/501577 mentions this issue: cmd/cover: error out if a requested source file contains a newline

@bcmills
Copy link
Contributor

bcmills commented Jun 7, 2023

This is mitigated in cmd/go, but a more robust fix should also reject newlines any time we would emit a line directive within tools or libraries. I've mailed some followup PUBLIC-track fixes to do that.

gopherbot pushed a commit that referenced this issue Jun 7, 2023
@gopherbot
cmd/go: fix TestScript/build_cwd_newline with CGO_ENABLED=0
e2b1c0b 
Updates #60167.

Change-Id: I3792682e80a3c48d78a3b9e647cc968a1d5c8f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/501575
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Jun 7, 2023
@gopherbot
cmd/cgo: error out if the source path used in line directives would c…
c482283 
…ontain a newline

cmd/cgo uses '//line' directives to map generated source
files back to the original source file and line nmubers.

The line directives have no way to escape newline characters,
so cmd/cgo must not be used if the line directives would contain
such characters.

Updates #60167.

Change-Id: I8581cea74d6c08f82e86ed87127e81252e1bf78c
Reviewed-on: https://go-review.googlesource.com/c/go/+/501576
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Jun 7, 2023
@gopherbot
cmd/cover: error out if a requested source file contains a newline
3d78c73 
cmd/cover uses '//line' directives to map instrumented source files
back to the original source file and line numbers.
Line directives have no way to escape newline characters, so cmd/cover
must not be used with source file paths that contain such characters.

Updates #60167.

Change-Id: I6dc039392d59fc3a5a6121ef6ca97b0ab0da5288
Reviewed-on: https://go-review.googlesource.com/c/go/+/501577
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jun 8, 2023
@gopherbot
go/printer: error out of Fprint when it would write a '//line' direct…
b9baf44 
…ive with a multiline file path

Line directives do not provide a way to escape newline characters, so
source file paths containing newlines must not be written in them.

Updates #60167.

Change-Id: I30f8b381cc7d1df6914c27591544edf424a4b634
Reviewed-on: https://go-review.googlesource.com/c/go/+/501578
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/501816 mentions this issue: [release-branch.go1.20] cmd/go: fix TestScript/build_cwd_newline with CGO_ENABLED=0

@gopherbot
Copy link

Change https://go.dev/cl/501817 mentions this issue: [release-branch.go1.20] cmd/cgo: error out if the source path used in line directives would contain a newline

@gopherbot
Copy link

Change https://go.dev/cl/501818 mentions this issue: [release-branch.go1.20] cmd/cover: error out if a requested source file contains a newline

@gopherbot
Copy link

Change https://go.dev/cl/501819 mentions this issue: [release-branch.go1.20] go/printer: error out of Fprint when it would write a '//line' directive with a multiline file path

@gopherbot
Copy link

Change https://go.dev/cl/501820 mentions this issue: [release-branch.go1.19] cmd/go: fix TestScript/build_cwd_newline with CGO_ENABLED=0

@gopherbot
Copy link

Change https://go.dev/cl/501821 mentions this issue: [release-branch.go1.19] cmd/cgo: error out if the source path used in line directives would contain a newline

@gopherbot
Copy link

Change https://go.dev/cl/501822 mentions this issue: [release-branch.go1.19] cmd/cover: error out if a requested source file contains a newline

@gopherbot
Copy link

Change https://go.dev/cl/501823 mentions this issue: [release-branch.go1.19] go/printer: error out of Fprint when it would write a '//line' directive with a multiline file path

gopherbot pushed a commit that referenced this issue Jun 13, 2023
@prattmic
[release-branch.go1.19] cmd/go: fix TestScript/build_cwd_newline with…
0f7675d 
… CGO_ENABLED=0

Updates #60515.
Updates #60167.

Change-Id: I3792682e80a3c48d78a3b9e647cc968a1d5c8f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/501575
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit e2b1c0b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501820
TryBot-Bypass: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Jun 13, 2023
@prattmic
[release-branch.go1.19] cmd/cgo: error out if the source path used in…
516ef2d 
… line directives would contain a newline

cmd/cgo uses '//line' directives to map generated source
files back to the original source file and line nmubers.

The line directives have no way to escape newline characters,
so cmd/cgo must not be used if the line directives would contain
such characters.

Updates #60515.
Updates #60167.

Change-Id: I8581cea74d6c08f82e86ed87127e81252e1bf78c
Reviewed-on: https://go-review.googlesource.com/c/go/+/501576
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
(cherry picked from commit c482283)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501821
TryBot-Bypass: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Jun 13, 2023
@prattmic
[release-branch.go1.19] cmd/cover: error out if a requested source fi…
6d44c15 
…le contains a newline

cmd/cover uses '//line' directives to map instrumented source files
back to the original source file and line numbers.
Line directives have no way to escape newline characters, so cmd/cover
must not be used with source file paths that contain such characters.

Updates #60515.
Updates #60167.

Change-Id: I6dc039392d59fc3a5a6121ef6ca97b0ab0da5288
Reviewed-on: https://go-review.googlesource.com/c/go/+/501577
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 3d78c73)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501822
gopherbot pushed a commit that referenced this issue Jun 13, 2023
@prattmic
[release-branch.go1.19] go/printer: error out of Fprint when it would…
0b0f86e 
… write a '//line' directive with a multiline file path

Line directives do not provide a way to escape newline characters, so
source file paths containing newlines must not be written in them.

Updates #60515.
Updates #60167.

Change-Id: I30f8b381cc7d1df6914c27591544edf424a4b634
Reviewed-on: https://go-review.googlesource.com/c/go/+/501578
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit d1087efa42ea0b0f011283a87d7a732cba51e4ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501823
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Jun 20, 2023
@gopherbot
[release-branch.go1.20] cmd/go: fix TestScript/build_cwd_newline with…
6c606fc 
… CGO_ENABLED=0

Updates #60516.
Updates #60167.

Change-Id: I3792682e80a3c48d78a3b9e647cc968a1d5c8f2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/501575
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit e2b1c0b)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501816
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
gopherbot pushed a commit that referenced this issue Jun 20, 2023
@gopherbot
[release-branch.go1.20] cmd/cgo: error out if the source path used in…
4719048 
… line directives would contain a newline

cmd/cgo uses '//line' directives to map generated source
files back to the original source file and line nmubers.

The line directives have no way to escape newline characters,
so cmd/cgo must not be used if the line directives would contain
such characters.

Updates #60516.
Updates #60167.

Change-Id: I8581cea74d6c08f82e86ed87127e81252e1bf78c
Reviewed-on: https://go-review.googlesource.com/c/go/+/501576
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
(cherry picked from commit c482283)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501817
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
gopherbot pushed a commit that referenced this issue Jun 20, 2023
@gopherbot
[release-branch.go1.20] cmd/cover: error out if a requested source fi…
4959424 
…le contains a newline

cmd/cover uses '//line' directives to map instrumented source files
back to the original source file and line numbers.
Line directives have no way to escape newline characters, so cmd/cover
must not be used with source file paths that contain such characters.

Updates #60516.
Updates #60167.

Change-Id: I6dc039392d59fc3a5a6121ef6ca97b0ab0da5288
Reviewed-on: https://go-review.googlesource.com/c/go/+/501577
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 3d78c73)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501818
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
gopherbot pushed a commit that referenced this issue Jun 20, 2023
@gopherbot
[release-branch.go1.20] go/printer: error out of Fprint when it would…
d51e322 
… write a '//line' directive with a multiline file path

Line directives do not provide a way to escape newline characters, so
source file paths containing newlines must not be written in them.

Updates #60516.
Updates #60167.

Change-Id: I30f8b381cc7d1df6914c27591544edf424a4b634
Reviewed-on: https://go-review.googlesource.com/c/go/+/501578
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit d1087efa42ea0b0f011283a87d7a732cba51e4ad)
Reviewed-on: https://go-review.googlesource.com/c/go/+/501819
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
@ArchanaWind
Copy link

Hi,

Could you please confirm Is this issue present in go1.17.13

Thanks in advance

@bcmills
Copy link
Contributor

bcmills commented Jun 22, 2023

Yes — see https://pkg.go.dev/vuln/GO-2023-1839.

@ArchanaWind
Copy link

ArchanaWind commented Jun 23, 2023 •
edited

Hi @bcmills,

Thank you for the quick response,

I have backported the CVE fix to the go1.17.13 version, Could you please review the attached patch and provide you're comments

Thanks in advance
CVE-2023-29402.patch

@ianlancetaylor
Copy link
Contributor

@ArchanaWind Just to be clear, Go 1.17 is no longer supported. The only currently supported versions of Go are Go 1.19 and Go 1.20.

@bcmills
Copy link
Contributor

bcmills commented Jun 23, 2023

@ArchanaWind, I've got a long backlog of code reviews at the moment. Given that only the two most recent Go releases are supported by the Go team, I do not personally plan to review patches for the unsupported releases.

@ArchanaWind
Copy link

@ianlancetaylor & @bcmills Fine Thank You

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done. release-blocker Security
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
7 participants
@dmitshur @ianlancetaylor @rolandshoemaker @bcmills @gopherbot @cherrymui @ArchanaWind