On Fri, May 21, 2021 at 3:30 PM Moritz Muehlenhoff wrote:
> Package: golang-github-containers-image
> Severity: important
> Tags: security
> X-Debbugs-Cc: Debian Security Team
>
> This was assigned CVE-2021-20291:
>
> https://github.com/containers/storage/commit/306fcabc964470e4b3b87a43a8f6b7d698209ee1
>
>
>
Moritz,
here is some more context on severity impact of this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1939485#c23
> For applications like podman the affect is minimal - podman pull and it
seemingly waits to download the image forever, cancel and the affect is
negated. For something like crio the affect is more severe, with the
malicious image locking the service up, BUT it is still somewhat
responsive. The service then must be killed before returning back to
normal.
The referenced commit changes the containers/storage code to no longer
fork()/exec() out to the xz executable, but instead use the golang-native
implementation of golang-github-ulikunitz-xz-dev, which addresses the
deadlock situation by avoiding awkward unix process coordination.
Upstream switched to version 0.5.10, whereas we only have 0.5.6 in Debian.
That version is at least susceptible against:
- https://github.com/ulikunitz/xz/issues/35 - CVE-2020-16845
-
https://github.com/ulikunitz/xz/commit/69c6093c7b2397b923acf82cb378f55ab2652b9b
(similar to/same as CVE-2020-16845)
- https://github.com/ulikunitz/xz/issues/40 -- no CVE, but could also lead
to a DoS situation, I guess
Given the limited impact of this issue (it could leave podman hanging,
leading to a DoS situation in some scenarios), the absence of any unit
tests, and the fact that we'd need to rebuild podman and friends anyways,
I'm pondering whether making this change is worth the risk. Moritz, what do
you think?
If we decided to proceed, the debdiff would look like this:
diff --git a/debian/changelog b/debian/changelog
index 837efeeb1..ad17e4867 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+golang-github-containers-storage (1.24.8+dfsg1-2) unstable; urgency=high
+
+ * Build against system copy of golang-github-ulikunitz-xz-dev,
+Adresses: CVE-2021-20291, Closes: #988942
+
+ -- Reinhard Tartler Fri, 21 May 2021 16:04:46 -0400
+
golang-github-containers-storage (1.24.8+dfsg1-1) unstable; urgency=medium
* New upstream release, focused on targetted bugfixes for podman 3.0
diff --git a/debian/control b/debian/control
index 086dbcb3d..c5c362961 100644
--- a/debian/control
+++ b/debian/control
@@ -24,6 +24,7 @@ Build-Depends: debhelper-compat (= 11),
golang-github-pquerna-ffjson-dev,
golang-github-sirupsen-logrus-dev,
golang-github-stretchr-testify-dev,
+ golang-github-ulikunitz-xz-dev,
golang-github-vbatts-tar-split-dev,
golang-go (>> 2:1.14~~),
golang-go-patricia-dev,
diff --git a/debian/patches/CVE-2021-20291.patch
b/debian/patches/CVE-2021-20291.patch
new file mode 100644
index 0..f87427443
--- /dev/null
+++ b/debian/patches/CVE-2021-20291.patch
@@ -0,0 +1,212 @@
+From 306fcabc964470e4b3b87a43a8f6b7d698209ee1 Mon Sep 17 00:00:00 2001
+From: Nalin Dahyabhai
+Date: Wed, 17 Mar 2021 17:24:14 -0400
+Subject: [PATCH] Use an xz library instead of shelling out to xz for
+ decompression
+
+When decompressing layers compressed with xz, use a library rather than
+shelling out to the xz CLI.
+
+Signed-off-by: Nalin Dahyabhai
+
+--- a/go.mod
b/go.mod
+@@ -23,6 +23,7 @@
+ github.com/stretchr/testify v1.6.1
+ github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2
+ github.com/tchap/go-patricia v2.3.0+incompatible
++ github.com/ulikunitz/xz v0.5.10
+ github.com/vbatts/tar-split v0.11.1
+ golang.org/x/net v0.0.0-20191004110552-13f9640d40b9
+ golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3
+--- a/pkg/archive/archive.go
b/pkg/archive/archive.go
+@@ -9,7 +9,6 @@
+ "io"
+ "io/ioutil"
+ "os"
+- "os/exec"
+ "path/filepath"
+ "runtime"
+ "strings"
+@@ -18,7 +17,6 @@
+
+ "github.com/containers/storage/pkg/fileutils"
+ "github.com/containers/storage/pkg/idtools"
+- "github.com/containers/storage/pkg/ioutils"
+ "github.com/containers/storage/pkg/pools"
+ "github.com/containers/storage/pkg/promise"
+ "github.com/containers/storage/pkg/system"
+@@ -26,6 +24,7 @@
+ rsystem "github.com/opencontainers/runc/libcontainer/system"
+ "github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
++ "github.com/ulikunitz/xz"
+ )
+
+ type (
+@@ -167,12 +166,6 @@
+ return Uncompressed
+ }
+
+-func xzDecompress(archive io.Reader) (io.ReadCloser, <-chan struct{},
error) {
+- args := []string{"xz", "-d", "-c", "-q"}
+-
+- return cmdStream(exec.Command(args[0], args[1:]...), archive)
+-}
+-
+ // DecompressStream decompresses the archive and returns a ReaderCloser
with the decompressed archive.
+ func DecompressStream(archive io.Reader) (io.ReadCloser, error) {
+ p := pools.BufioReader32KPool
+@@ -205,15 +198,12 @@
+ readBufWrapper := p
