From e0ff7cccc3cac73da41ec9ef007b0e4e97c55d01 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 02:50:52 +1100 Subject: [PATCH 1/3] vendor: update to github.com/vbatts/tar-split@v0.10.2 Update to the latest version of tar-split, which includes a change to fix a memory exhaustion issue where a malformed image could cause the Docker daemon to crash. * tar: asm: store padding in chunks to avoid memory exhaustion Fixes: CVE-2017-14992 Signed-off-by: Aleksa Sarai --- vendor.conf | 2 +- vendor/github.com/vbatts/tar-split/README.md | 3 +- .../vbatts/tar-split/tar/asm/disassemble.go | 43 ++++++++++++------- 3 files changed, 31 insertions(+), 17 deletions(-) diff --git a/vendor.conf b/vendor.conf index bec25b7e59..74a81884eb 100644 --- a/vendor.conf +++ b/vendor.conf @@ -55,7 +55,7 @@ github.com/miekg/dns 75e6e86cc601825c5dbcd4e0c209eab180997cd7 # get graph and distribution packages github.com/docker/distribution edc3ab29cdff8694dd6feb85cfeb4b5f1b38ed9c -github.com/vbatts/tar-split v0.10.1 +github.com/vbatts/tar-split v0.10.2 github.com/opencontainers/go-digest a6d0ee40d4207ea02364bd3b9e8e77b9159ba1eb # get go-zfs packages diff --git a/vendor/github.com/vbatts/tar-split/README.md b/vendor/github.com/vbatts/tar-split/README.md index 4c544d823f..03e3ec4308 100644 --- a/vendor/github.com/vbatts/tar-split/README.md +++ b/vendor/github.com/vbatts/tar-split/README.md @@ -1,6 +1,7 @@ # tar-split [![Build Status](https://travis-ci.org/vbatts/tar-split.svg?branch=master)](https://travis-ci.org/vbatts/tar-split) +[![Go Report Card](https://goreportcard.com/badge/github.com/vbatts/tar-split)](https://goreportcard.com/report/github.com/vbatts/tar-split) Pristinely disassembling a tar archive, and stashing needed raw bytes and offsets to reassemble a validating original archive. @@ -50,7 +51,7 @@ For example stored sparse files that have "holes" in them, will be read as a contiguous file, though the archive contents may be recorded in sparse format. Therefore when adding the file payload to a reassembled tar, to achieve identical output, the file payload would need be precisely re-sparsified. This -is not something I seek to fix imediately, but would rather have an alert that +is not something I seek to fix immediately, but would rather have an alert that precise reassembly is not possible. (see more http://www.gnu.org/software/tar/manual/html_node/Sparse-Formats.html) diff --git a/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go b/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go index 54ef23aed3..009b3f5d81 100644 --- a/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go +++ b/vendor/github.com/vbatts/tar-split/tar/asm/disassemble.go @@ -2,7 +2,6 @@ package asm import ( "io" - "io/ioutil" "github.com/vbatts/tar-split/archive/tar" "github.com/vbatts/tar-split/tar/storage" @@ -119,20 +118,34 @@ func NewInputTarStream(r io.Reader, p storage.Packer, fp storage.FilePutter) (io } } - // it is allowable, and not uncommon that there is further padding on the - // end of an archive, apart from the expected 1024 null bytes. - remainder, err := ioutil.ReadAll(outputRdr) - if err != nil && err != io.EOF { - pW.CloseWithError(err) - return - } - _, err = p.AddEntry(storage.Entry{ - Type: storage.SegmentType, - Payload: remainder, - }) - if err != nil { - pW.CloseWithError(err) - return + // It is allowable, and not uncommon that there is further padding on + // the end of an archive, apart from the expected 1024 null bytes. We + // do this in chunks rather than in one go to avoid cases where a + // maliciously crafted tar file tries to trick us into reading many GBs + // into memory. + const paddingChunkSize = 1024 * 1024 + var paddingChunk [paddingChunkSize]byte + for { + var isEOF bool + n, err := outputRdr.Read(paddingChunk[:]) + if err != nil { + if err != io.EOF { + pW.CloseWithError(err) + return + } + isEOF = true + } + _, err = p.AddEntry(storage.Entry{ + Type: storage.SegmentType, + Payload: paddingChunk[:n], + }) + if err != nil { + pW.CloseWithError(err) + return + } + if isEOF { + break + } } pW.Close() }() From 2f8d3e1c33f77187c68893803018756d43daff15 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 03:29:49 +1100 Subject: [PATCH 2/3] internal: testutil: add DevZero helper This helper acts like /dev/zero (outputs \x00 indefinitely) in an OS-independent fashion. This ensures we don't need to special-case around Windows in tests that want to open /dev/zero. Signed-off-by: Aleksa Sarai --- internal/testutil/helpers.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/testutil/helpers.go b/internal/testutil/helpers.go index a76056924e..287b3cb48a 100644 --- a/internal/testutil/helpers.go +++ b/internal/testutil/helpers.go @@ -1,6 +1,8 @@ package testutil import ( + "io" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -11,3 +13,15 @@ func ErrorContains(t require.TestingT, err error, expectedError string, msgAndAr require.Error(t, err, msgAndArgs...) assert.Contains(t, err.Error(), expectedError, msgAndArgs...) } + +// DevZero acts like /dev/zero but in an OS-independent fashion. +var DevZero io.Reader = devZero{} + +type devZero struct{} + +func (d devZero) Read(p []byte) (n int, err error) { + for i := 0; i < len(p); i++ { + p[i] = '\x00' + } + return len(p), nil +} From 0a13f827a10d3bf61744d9b3f7165c5885a39c5d Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 03:30:47 +1100 Subject: [PATCH 3/3] image: add import test for CVE-2017-14992 To ensure that we don't revert CVE-2017-14992, add a test that is quite similar to that upstream tar-split test (create an empty archive with lots of junk and make sure the daemon doesn't crash). Signed-off-by: Aleksa Sarai --- integration/image/import_test.go | 36 ++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 integration/image/import_test.go diff --git a/integration/image/import_test.go b/integration/image/import_test.go new file mode 100644 index 0000000000..955891f288 --- /dev/null +++ b/integration/image/import_test.go @@ -0,0 +1,36 @@ +package image + +import ( + "archive/tar" + "bytes" + "context" + "io" + "testing" + + "github.com/docker/docker/api/types" + "github.com/docker/docker/integration/util/request" + "github.com/docker/docker/internal/testutil" +) + +// Ensure we don't regress on CVE-2017-14992. +func TestImportExtremelyLargeImageWorks(t *testing.T) { + client := request.NewAPIClient(t) + + // Construct an empty tar archive with about 8GB of junk padding at the + // end. This should not cause any crashes (the padding should be mostly + // ignored). + var tarBuffer bytes.Buffer + tw := tar.NewWriter(&tarBuffer) + if err := tw.Close(); err != nil { + t.Fatal(err) + } + imageRdr := io.MultiReader(&tarBuffer, io.LimitReader(testutil.DevZero, 8*1024*1024*1024)) + + _, err := client.ImageImport(context.Background(), + types.ImageImportSource{Source: imageRdr, SourceName: "-"}, + "test1234:v42", + types.ImageImportOptions{}) + if err != nil { + t.Fatal(err) + } +}