From b9775006bf71ba57bd488d7faf097d18d9979b7b Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 02:12:49 +1100 Subject: [PATCH 1/3] *: move tar_benchmark to cmd/tar-split/ This fixes a new go-vet(1) error which has surfaced in Go HEAD. $ go vet ./... go build github.com/vbatts/tar-split: no non-test Go files in /home/travis/gopath/src/github.com/vbatts/tar-split Signed-off-by: Aleksa Sarai --- tar_benchmark_test.go => cmd/tar-split/tar_benchmark_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename tar_benchmark_test.go => cmd/tar-split/tar_benchmark_test.go (94%) diff --git a/tar_benchmark_test.go b/cmd/tar-split/tar_benchmark_test.go similarity index 94% rename from tar_benchmark_test.go rename to cmd/tar-split/tar_benchmark_test.go index d946f2a..f318645 100644 --- a/tar_benchmark_test.go +++ b/cmd/tar-split/tar_benchmark_test.go @@ -1,4 +1,4 @@ -package tartest +package main import ( "io" @@ -11,7 +11,7 @@ import ( ourTar "github.com/vbatts/tar-split/archive/tar" ) -var testfile = "./archive/tar/testdata/sparse-formats.tar" +var testfile = "../../archive/tar/testdata/sparse-formats.tar" func BenchmarkUpstreamTar(b *testing.B) { for n := 0; n < b.N; n++ { From 3d9db48dbe79715fe8ad9db8696e4d8a7c53c18a Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 00:05:46 +1100 Subject: [PATCH 2/3] tar: asm: store padding in chunks to avoid memory exhaustion Previously, we would read the entire padding in a given archive into memory in order to store it in the packer. This would cause memory exhaustion if a malicious archive was crafted with very large amounts of padding. Since a given SegmentType is reconstructed losslessly, we can simply chunk up any padding into large segments to avoid this problem. Use a reasonable default of 1MiB to avoid changing the tar-split.json of existing archives that are not malformed. Fixes: CVE-2017-14992 Signed-off-by: Aleksa Sarai --- tar/asm/disassemble.go | 43 +++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/tar/asm/disassemble.go b/tar/asm/disassemble.go index 54ef23a..009b3f5 100644 --- a/tar/asm/disassemble.go +++ b/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 99430a8454c500c908a5ce931b875fb4d38b6204 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 8 Nov 2017 00:34:34 +1100 Subject: [PATCH 3/3] tar: asm: add an excess padding test case To ensure we don't have regressions in our padding fix, add a test case that attempts to crash the test by creating 20GB of random junk padding. Signed-off-by: Aleksa Sarai --- tar/asm/disassemble_test.go | 72 +++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tar/asm/disassemble_test.go diff --git a/tar/asm/disassemble_test.go b/tar/asm/disassemble_test.go new file mode 100644 index 0000000..4e45714 --- /dev/null +++ b/tar/asm/disassemble_test.go @@ -0,0 +1,72 @@ +package asm + +import ( + "archive/tar" + "fmt" + "io" + "io/ioutil" + "os" + "testing" + + "github.com/vbatts/tar-split/tar/storage" +) + +// This test failing causes the binary to crash due to memory overcommitment. +func TestLargeJunkPadding(t *testing.T) { + pR, pW := io.Pipe() + + // Write a normal tar file into the pipe and then load it full of junk + // bytes as padding. We have to do this in a goroutine because we can't + // store 20GB of junk in-memory. + go func() { + // Empty archive. + tw := tar.NewWriter(pW) + if err := tw.Close(); err != nil { + pW.CloseWithError(err) + t.Fatal(err) + return + } + + // Write junk. + const ( + junkChunkSize = 64 * 1024 * 1024 + junkChunkNum = 20 * 16 + ) + devZero, err := os.Open("/dev/zero") + if err != nil { + pW.CloseWithError(err) + t.Fatal(err) + return + } + defer devZero.Close() + for i := 0; i < junkChunkNum; i++ { + if i%32 == 0 { + fmt.Fprintf(os.Stderr, "[TestLargeJunkPadding] junk chunk #%d/#%d\n", i, junkChunkNum) + } + if _, err := io.CopyN(pW, devZero, junkChunkSize); err != nil { + pW.CloseWithError(err) + t.Fatal(err) + return + } + } + + fmt.Fprintln(os.Stderr, "[TestLargeJunkPadding] junk chunk finished") + pW.Close() + }() + + // Disassemble our junk file. + nilPacker := storage.NewJSONPacker(ioutil.Discard) + rdr, err := NewInputTarStream(pR, nilPacker, nil) + if err != nil { + t.Fatal(err) + } + + // Copy the entire rdr. + _, err = io.Copy(ioutil.Discard, rdr) + if err != nil { + t.Fatal(err) + } + + // At this point, if we haven't crashed then we are not vulnerable to + // CVE-2017-14992. +}