diff --git a/internal/config/plugin_installer.go b/internal/config/plugin_installer.go index d892cde910..e0c80e60c8 100644 --- a/internal/config/plugin_installer.go +++ b/internal/config/plugin_installer.go @@ -420,7 +420,7 @@ func (pv *PluginVersion) DownloadAndInstall(out io.Writer) error { return err } targetDir := filepath.Join(ConfigDir, "plug", pv.pack.Name) - dirPerm := os.FileMode(0755) + dirPerm := util.DirMode if err = os.MkdirAll(targetDir, dirPerm); err != nil { return err } diff --git a/internal/util/util.go b/internal/util/util.go index 9838b025fb..478aef0af6 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -51,6 +51,9 @@ var ( // To be used for file writes before umask is applied const FileMode os.FileMode = 0666 +// To be used for directory creation before umask is applied +const DirMode os.FileMode = 0755 + const BackupSuffix = ".micro-backup" const OverwriteFailMsg = `An error occurred while writing to the file: @@ -629,6 +632,9 @@ func String(s []byte) string { return string(s) } +// maxUnzipSize is the most Unzip will extract from a single archive +const maxUnzipSize = 1 << 30 + // Unzip unzips a file to given folder func Unzip(src, dest string) error { r, err := zip.OpenReader(src) @@ -637,7 +643,9 @@ func Unzip(src, dest string) error { } defer r.Close() - os.MkdirAll(dest, 0755) + os.MkdirAll(dest, DirMode) + + var remaining int64 = maxUnzipSize // Closure to address file descriptors issue with all the deferred .Close() methods extractAndWriteFile := func(f *zip.File) error { @@ -654,20 +662,28 @@ func Unzip(src, dest string) error { return fmt.Errorf("illegal file path: %s", path) } + // Check for zip bombs (output far larger than the archive). archive/zip + // refuses to read past the size declared in an entry's header, so the + // declared size is what gets checked against the budget. + if f.UncompressedSize64 > uint64(remaining) { + return fmt.Errorf("file too large: %s", path) + } + if f.FileInfo().IsDir() { os.MkdirAll(path, f.Mode()) } else { - os.MkdirAll(filepath.Dir(path), f.Mode()) + os.MkdirAll(filepath.Dir(path), DirMode) f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, f.Mode()) if err != nil { return err } defer f.Close() - _, err = io.Copy(f, rc) + n, err := io.Copy(f, rc) if err != nil { return err } + remaining -= n } return nil } diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 454b5c7963..801f7e4ca1 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -1,6 +1,10 @@ package util import ( + "archive/zip" + "bytes" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -31,3 +35,63 @@ func TestSliceVisualEnd(t *testing.T) { assert.Equal(t, []byte("ello"), slc) assert.Equal(t, 0, n) } + +func writeZip(t *testing.T, add func(w *zip.Writer)) string { + path := filepath.Join(t.TempDir(), "test.zip") + f, err := os.Create(path) + assert.NoError(t, err) + w := zip.NewWriter(f) + add(w) + assert.NoError(t, w.Close()) + assert.NoError(t, f.Close()) + return path +} + +func TestUnzip(t *testing.T) { + src := writeZip(t, func(w *zip.Writer) { + fw, _ := w.Create("dir/hello.txt") + fw.Write([]byte("hello")) + }) + dest := t.TempDir() + + assert.NoError(t, Unzip(src, dest)) + data, err := os.ReadFile(filepath.Join(dest, "dir", "hello.txt")) + assert.NoError(t, err) + assert.Equal(t, "hello", string(data)) +} + +func TestUnzipTooLarge(t *testing.T) { + // A header that claims more than the limit, backed by only a few real bytes + src := writeZip(t, func(w *zip.Writer) { + fw, _ := w.CreateRaw(&zip.FileHeader{ + Name: "bomb.txt", + Method: zip.Store, + CompressedSize64: 5, + UncompressedSize64: maxUnzipSize + 1, + }) + fw.Write([]byte("hello")) + }) + dest := t.TempDir() + + err := Unzip(src, dest) + assert.EqualError(t, err, "file too large: "+filepath.Join(dest, "bomb.txt")) + _, err = os.Stat(filepath.Join(dest, "bomb.txt")) + assert.True(t, os.IsNotExist(err)) +} + +func TestUnzipUnderstatedSize(t *testing.T) { + // A header that declares 10 bytes in front of 300 real ones. Unzip trusts + // the declared size because archive/zip refuses to read past it. + payload := bytes.Repeat([]byte("micro "), 50) + src := writeZip(t, func(w *zip.Writer) { + fw, _ := w.CreateRaw(&zip.FileHeader{ + Name: "bomb.txt", + Method: zip.Store, + CompressedSize64: uint64(len(payload)), + UncompressedSize64: 10, + }) + fw.Write(payload) + }) + + assert.Equal(t, zip.ErrFormat, Unzip(src, t.TempDir())) +}