From 50de4986e962669a3448f7d103e2b39c2c9be45d Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Thu, 3 Sep 2026 19:42:36 +0300 Subject: [PATCH 1/2] util: Cap the amount of data Unzip extracts Unzip copied every archive entry with io.Copy and no bound on the output, so a small crafted archive handed to a plugin could expand until the disk filled. The function already rejects entries that escape the destination directory; this adds the matching check for size. Extraction now has a 1 GiB budget per archive. Each entry's declared uncompressed size is checked against what is left before anything is written, and the bytes actually written are subtracted afterwards. archive/zip refuses to read past the size declared in an entry's header, so the declared size is a real upper bound and no second limit is needed on the copy. Exceeding the budget returns an error in the same style as the traversal check. The new test for a plain archive also showed that Unzip created the parent directory of a file entry with the file's own mode. Archives without directory entries, such as ones written by Go's archive/zip, ended up with an unsearchable 0644 directory on Unix and every file inside it failed to open. Parent directories now get 0755, the same as the destination directory. Fixes #4161 --- internal/util/util.go | 17 ++++++++-- internal/util/util_test.go | 64 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/internal/util/util.go b/internal/util/util.go index 9838b025fb..bad7a712be 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -629,6 +629,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) @@ -639,6 +642,8 @@ func Unzip(src, dest string) error { os.MkdirAll(dest, 0755) + var remaining int64 = maxUnzipSize + // Closure to address file descriptors issue with all the deferred .Close() methods extractAndWriteFile := func(f *zip.File) error { rc, err := f.Open() @@ -654,20 +659,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), 0755) 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())) +} From ff05d1a7129cedb8b61d8756f7dbb59774f0b338 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Sat, 5 Sep 2026 00:35:34 +0300 Subject: [PATCH 2/2] util: Name the directory mode used for extraction Unzip and the plugin installer both created directories with a bare 0755, so give it a DirMode constant next to FileMode and use it in all three places. --- internal/config/plugin_installer.go | 2 +- internal/util/util.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) 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 bad7a712be..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: @@ -640,7 +643,7 @@ func Unzip(src, dest string) error { } defer r.Close() - os.MkdirAll(dest, 0755) + os.MkdirAll(dest, DirMode) var remaining int64 = maxUnzipSize @@ -669,7 +672,7 @@ func Unzip(src, dest string) error { if f.FileInfo().IsDir() { os.MkdirAll(path, f.Mode()) } else { - os.MkdirAll(filepath.Dir(path), 0755) + 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