diff --git a/common/test-fixtures/decompress-tar/outside_parent.tar b/common/test-fixtures/decompress-tar/outside_parent.tar new file mode 100644 index 000000000..f08df1e65 Binary files /dev/null and b/common/test-fixtures/decompress-tar/outside_parent.tar differ diff --git a/post-processor/vagrant/virtualbox.go b/post-processor/vagrant/virtualbox.go index 6b01a9c8c..3efda5e9f 100644 --- a/post-processor/vagrant/virtualbox.go +++ b/post-processor/vagrant/virtualbox.go @@ -133,7 +133,15 @@ func DecompressOva(dir, src string) error { if hdr == nil || err == io.EOF { break } + if err != nil { + return err + } + // We use the fileinfo to get the file name because we are not + // expecting path information as from the tar header. It's important + // that we not use the path name from the tar header without checking + // for the presence of `..`. If we accidentally allow for that, we can + // open ourselves up to a path traversal vulnerability. info := hdr.FileInfo() // Shouldn't be any directories, skip them diff --git a/post-processor/vagrant/virtualbox_test.go b/post-processor/vagrant/virtualbox_test.go index c3815ce77..5082c9388 100644 --- a/post-processor/vagrant/virtualbox_test.go +++ b/post-processor/vagrant/virtualbox_test.go @@ -1,9 +1,27 @@ package vagrant import ( + "io/ioutil" + "os" + "path/filepath" "testing" + + "github.com/stretchr/testify/assert" ) func TestVBoxProvider_impl(t *testing.T) { var _ Provider = new(VBoxProvider) } + +func TestDecomressOVA(t *testing.T) { + td, err := ioutil.TempDir("", "pp-vagrant-virtualbox") + assert.NoError(t, err) + fixture := "../../common/test-fixtures/decompress-tar/outside_parent.tar" + err = DecompressOva(td, fixture) + assert.NoError(t, err) + _, err = os.Stat(filepath.Join(filepath.Base(td), "demo.poc")) + assert.Error(t, err) + _, err = os.Stat(filepath.Join(td, "demo.poc")) + assert.NoError(t, err) + os.RemoveAll(td) +}