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

[release/1.5] Update go 1.18.7, addresses CVE-2022-2879, CVE-2022-2880, CVE-2022-41715 #7476

Merged
merged 10 commits into from
Oct 20, 2022

Conversation

thaJeztah
Copy link
Member

From the mailing list:

We have just released Go versions 1.19.2 and 1.18.7, minor point releases.

These minor releases include 3 security fixes following the security policy:

  • archive/tar: unbounded memory consumption when reading headers

    Reader.Read did not set a limit on the maximum size of file headers. A maliciously crafted archive could cause Read to allocate unbounded amounts of memory, potentially causing resource exhaustion or panics. Reader.Read now limits the maximum size of header blocks to 1 MiB.

    Thanks to Adam Korczynski (ADA Logics) and OSS-Fuzz for reporting this issue.

    This is CVE-2022-2879 and Go issue https://go.dev/issue/54853.

  • net/http/httputil: ReverseProxy should not forward unparseable query parameters

    Requests forwarded by ReverseProxy included the raw query parameters from the inbound request, including unparseable parameters rejected by net/http. This could permit query parameter smuggling when a Go proxy forwards a parameter with an unparseable value.

    ReverseProxy will now sanitize the query parameters in the forwarded query when the outbound request's Form field is set after the ReverseProxy.Director function returns, indicating that the proxy has parsed the query parameters. Proxies which do not parse query parameters continue to forward the original query parameters unchanged.

    Thanks to Gal Goldstein (Security Researcher, Oxeye) and Daniel Abeles (Head of Research, Oxeye) for reporting this issue.

    This is CVE-2022-2880 and Go issue https://go.dev/issue/54663.

  • regexp/syntax: limit memory used by parsing regexps

    The parsed regexp representation is linear in the size of the input, but in some cases the constant factor can be as high as 40,000, making relatively small regexps consume much larger amounts of memory.

    Each regexp being parsed is now limited to a 256 MB memory footprint. Regular expressions whose representation would use more space than that are now rejected. Normal use of regular expressions is unaffected.

    Thanks to Adam Korczynski (ADA Logics) and OSS-Fuzz for reporting this issue.

    This is CVE-2022-41715 and Go issue https://go.dev/issue/55949.

View the release notes for more information: https://go.dev/doc/devel/release#go1.18.7

@thaJeztah
Copy link
Member Author

Hm... need to update golang-ci-lint

Running [/home/runner/golangci-lint-1.44.2-linux-amd64/golangci-lint run --out-format=github-actions --path-prefix=src/github.com/containerd/containerd --timeout=5m] in [/home/runner/work/containerd/containerd/src/github.com/containerd/containerd] ...
  panic: load embedded ruleguard rules: rules/rules.go:13: can't load fmt
  
  goroutine 1 [running]:
  github.com/go-critic/go-critic/checkers.init.22()
  	github.com/go-critic/go-critic@v0.6.2/checkers/embedded_rules.go:46 +0x4b4
  
  Error: golangci-lint exit with code 2
  Ran golangci-lint in 13248ms

@thaJeztah thaJeztah force-pushed the 1.5_bump_go_1.18.7 branch 2 times, most recently from 970971e to 0b007d9 Compare October 5, 2022 22:42
@thaJeztah
Copy link
Member Author

Added some commits to address the gofmt and linting changes (same as #7475); let's see if CI is happy with that 🤞

@thaJeztah
Copy link
Member Author

Okay; one more fix needed it looks like;

GO111MODULE=on go get github.com/cpuguy83/go-md2man/v2@v2.0.1
  shell: /bin/bash -e {0}
  env:
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    GOPATH: /home/runner/work/containerd/containerd
go: go.mod file not found in current directory or any parent directory.
	'go get' is no longer supported outside a module.
	To build and install a command, use 'go install' with a version,
	like 'go install example.com/cmd@latest'
	For more information, see https://golang.org/doc/go-get-install-deprecation
	or run 'go help get' or 'go help install'.
Error: Process completed with exit code 1.

@AkihiroSuda
Copy link
Member

CI failing

Run go install github.com/cpuguy83/go-md2man/v2@v2.0.1
$GOPATH/go.mod exists but should not
Error: Process completed with exit code 1.

https://github.com/containerd/containerd/actions/runs/3193263827/jobs/5211649800

@thaJeztah

This comment was marked as resolved.

@estesp
Copy link
Member

estesp commented Oct 12, 2022

Something weird happening and it doesn't make sense; removing the working-dir from several places in the actions yaml does match the other branches, but seems specifically for integration runs the action job is failing due to not being in the right directory or some path construction is incorrect? Didn't have much time to dig deeper, but definitely an issue there

@thaJeztah
Copy link
Member Author

Yes, it's odd (and these things are "fun" as you cannot try them locally 😞). Let me at least add a commit to update the golang.org/x/sys version to see if the macOS failure goes away (and open a separate PR for that); I'll use the same version as we used in the 1.6 branch (to not get too far ahead); taken from / same as 6ee5bb7

@thaJeztah

This comment was marked as resolved.

@thaJeztah thaJeztah force-pushed the 1.5_bump_go_1.18.7 branch 2 times, most recently from ae80711 to 219ebba Compare October 20, 2022 07:22
@thaJeztah
Copy link
Member Author

thaJeztah commented Oct 20, 2022

Okay remaining issues;

Linux:

sudo -E PATH=$PATH make integration EXTRA_TESTFLAGS=-no-criu TESTFLAGS_RACE=-race
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    TEST_RUNTIME: io.containerd.runtime.v1.linux
    RUNC_FLAVOR: runc
    GOTESTSUM_JUNITFILE: /home/runner/work/containerd/containerd/test-integration-serial-junit.xml
Error: An error occurred trying to start process '/bin/bash' with working directory '/home/runner/work/containerd/containerd/src/github.com/containerd/containerd'. No such file or directory
sudo -E PATH=$PATH make integration EXTRA_TESTFLAGS=-no-criu TESTFLAGS_RACE=-race
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    TEST_RUNTIME: io.containerd.runtime.v1.linux
    RUNC_FLAVOR: runc
    GOTESTSUM_JUNITFILE: /home/runner/work/containerd/containerd/test-integration-serial-junit.xml
Error: An error occurred trying to start process '/bin/bash' with working directory '/home/runner/work/containerd/containerd/src/github.com/containerd/containerd'. No such file or directory
sudo -E PATH=$PATH make integration EXTRA_TESTFLAGS=-no-criu TESTFLAGS_RACE=-race
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    TEST_RUNTIME: io.containerd.runc.v1
    RUNC_FLAVOR: runc
    GOTESTSUM_JUNITFILE: /home/runner/work/containerd/containerd/test-integration-serial-junit.xml
Error: An error occurred trying to start process '/bin/bash' with working directory '/home/runner/work/containerd/containerd/src/github.com/containerd/containerd'. No such file or directory
sudo -E PATH=$PATH make integration EXTRA_TESTFLAGS=-no-criu TESTFLAGS_RACE=-race
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    TEST_RUNTIME: io.containerd.runc.v2
    RUNC_FLAVOR: runc
    GOTESTSUM_JUNITFILE: /home/runner/work/containerd/containerd/test-integration-serial-junit.xml
Error: An error occurred trying to start process '/bin/bash' with working directory '/home/runner/work/containerd/containerd/src/github.com/containerd/containerd'. No such file or directory
sudo -E PATH=$PATH make integration EXTRA_TESTFLAGS=-no-criu TESTFLAGS_RACE=-race
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /opt/hostedtoolcache/go/1.18.7/x64
    TEST_RUNTIME: io.containerd.runc.v2
    RUNC_FLAVOR: crun
    GOTESTSUM_JUNITFILE: /home/runner/work/containerd/containerd/test-integration-serial-junit.xml
Error: An error occurred trying to start process '/bin/bash' with working directory '/home/runner/work/containerd/containerd/src/github.com/containerd/containerd'. No such file or directory

macOS (during installing gotestsum):

sudo -E PATH=$PATH script/setup/install-gotestsum
  shell: /bin/bash -e {0}
  env:
    GOTEST: gotestsum --
    GOROOT: /Users/runner/hostedtoolcache/go/1.18.7/x64

go: downloading golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
go: downloading golang.org/x/sync v0.0.0-20190423024810-112230192c58
# golang.org/x/sys/unix
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/syscall_darwin.1_13.go:29:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.1_13.go:27:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.1_13.go:40:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:28:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:43:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:59:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:75:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:90:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:105:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:121:3: //go:linkname must refer to declared function or variable
Error: ../../../go/pkg/mod/golang.org/x/sys@v0.0.0-20201009025420-dfb3f7c4e634/unix/zsyscall_darwin_amd64.go:121:3: too many errors
Error: Process completed with exit code 2.

Which I can reproduce locally; looks like gotestsum is the one failing because it depends on an old version of golang.org/x/sys, so we need

cpuguy83 and others added 10 commits October 20, 2022 13:12
The current release of gotestsum is missing timestamps in the junit
data, which makes it difficult to import in an external system later.

gotestyourself/gotestsum@012a85e
includes the necessary changes to add the timestamp for the test run to
the junit output.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
(cherry picked from commit bfbebf0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
removes golang.org/x/cryto dependency:
full diff: gotestyourself/gotestsum@012a85e...1a94380

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 603962b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Same as previous commit, but a release was tagged;

gotestyourself/gotestsum@1a94380...v1.7.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 18d6cc1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
https://go.dev/doc/go-get-install-deprecation

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
(cherry picked from commit 6b0e241)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
it looks like golangci-lint's gofmt check uses go1.19 rules, despite
being run with go1.18.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
(cherry picked from commit 3a3f43f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For compatibility with Go 1.18

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
CVE-2022-41715

From the mailing list:

We have just released Go versions 1.19.2 and 1.18.7, minor point releases.

These minor releases include 3 security fixes following the security policy:

- archive/tar: unbounded memory consumption when reading headers

  Reader.Read did not set a limit on the maximum size of file headers.
  A maliciously crafted archive could cause Read to allocate unbounded
  amounts of memory, potentially causing resource exhaustion or panics.
  Reader.Read now limits the maximum size of header blocks to 1 MiB.

  Thanks to Adam Korczynski (ADA Logics) and OSS-Fuzz for reporting this issue.

  This is CVE-2022-2879 and Go issue https://go.dev/issue/54853.

- net/http/httputil: ReverseProxy should not forward unparseable query parameters

  Requests forwarded by ReverseProxy included the raw query parameters from the
  inbound request, including unparseable parameters rejected by net/http. This
  could permit query parameter smuggling when a Go proxy forwards a parameter
  with an unparseable value.

  ReverseProxy will now sanitize the query parameters in the forwarded query
  when the outbound request's Form field is set after the ReverseProxy.Director
  function returns, indicating that the proxy has parsed the query parameters.
  Proxies which do not parse query parameters continue to forward the original
  query parameters unchanged.

  Thanks to Gal Goldstein (Security Researcher, Oxeye) and
  Daniel Abeles (Head of Research, Oxeye) for reporting this issue.

  This is CVE-2022-2880 and Go issue https://go.dev/issue/54663.

- regexp/syntax: limit memory used by parsing regexps

  The parsed regexp representation is linear in the size of the input,
  but in some cases the constant factor can be as high as 40,000,
  making relatively small regexps consume much larger amounts of memory.

  Each regexp being parsed is now limited to a 256 MB memory footprint.
  Regular expressions whose representation would use more space than that
  are now rejected. Normal use of regular expressions is unaffected.

  Thanks to Adam Korczynski (ADA Logics) and OSS-Fuzz for reporting this issue.

  This is CVE-2022-41715 and Go issue https://go.dev/issue/55949.

View the release notes for more information: https://go.dev/doc/devel/release#go1.18.7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: rongfu.leng <rongfu.leng@daocloud.io>
(cherry picked from commit 047e684)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

Whoop! It's green now! @kzys @estesp

I should say that I haven't tried running CI on each intermediate commit (in case we need to bisect); I think we should be fine, but its possible that some of the last commits should "technically" be in a different order.

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kzys kzys merged commit ebba784 into containerd:release/1.5 Oct 20, 2022
@thaJeztah thaJeztah deleted the 1.5_bump_go_1.18.7 branch October 20, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants