From a2eb99d26c50738933d2a15ed53d2ade6f351197 Mon Sep 17 00:00:00 2001 From: SABITHSAHEB Date: Sat, 20 Jun 2026 15:59:10 +0530 Subject: [PATCH] Sanitize attacker-controlled file modes in plugin extraction Signed-off-by: SABITHSAHEB --- internal/plugin/installer/extractor.go | 11 +++- .../plugin/installer/http_installer_test.go | 61 ++++++++++++++++++- internal/plugin/installer/oci_installer.go | 2 +- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/internal/plugin/installer/extractor.go b/internal/plugin/installer/extractor.go index 3af56ab56..273652f8d 100644 --- a/internal/plugin/installer/extractor.go +++ b/internal/plugin/installer/extractor.go @@ -123,6 +123,15 @@ func cleanJoin(root, dest string) (string, error) { return filepath.ToSlash(newpath), nil } +// sanitizeArchiveMode normalizes a file mode taken from an untrusted plugin +// archive header into a safe permission set. Plugin files are extracted and +// then executed, so setuid/setgid/sticky and group/other write bits are always +// dropped (regardless of the process umask) while the owner execute bit is +// preserved, mirroring how Helm expands charts (pkg/chart/v2/util/expand.go). +func sanitizeArchiveMode(mode int64) os.FileMode { + return os.FileMode(mode).Perm() & 0o755 +} + // Extract extracts compressed archives // // Implements Extractor. @@ -161,7 +170,7 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { return err } - outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, sanitizeArchiveMode(header.Mode)) if err != nil { return err } diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index efbca90c9..1866d03f0 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -270,9 +270,11 @@ func TestExtract(t *testing.T) { t.Fatalf("Did not expect error but got error: %v", err) } - // Calculate expected permissions after umask is applied + // Calculate expected permissions after umask is applied. README.md ships as + // 0777 in the archive but is sanitized to 0755 on extraction (group/other + // write stripped) before the umask is applied; see sanitizeArchiveMode. expectedPluginYAMLPerm := os.FileMode(0600 &^ currentUmask) - expectedReadmePerm := os.FileMode(0777 &^ currentUmask) + expectedReadmePerm := os.FileMode(0755 &^ currentUmask) pluginYAMLFullPath := filepath.Join(tempDir, "plugin.yaml") if info, err := os.Stat(pluginYAMLFullPath); err != nil { @@ -297,6 +299,61 @@ func TestExtract(t *testing.T) { } } +// TestExtractSanitizesFileMode verifies that setuid/setgid/sticky and +// group/other write bits from an untrusted plugin archive are dropped on +// extraction (independent of umask), while the owner execute bit is preserved. +func TestExtractSanitizesFileMode(t *testing.T) { + defer syscall.Umask(syscall.Umask(0)) // umask 0: exercise the sanitizer, not the umask + + modes := map[string]int64{ + "setuid-binary": 0o6755, // setuid + setgid + rwxr-xr-x + "sticky-file": 0o1777, // sticky + rwxrwxrwx + "world-writable": 0o666, // rw-rw-rw- + } + + var tarbuf bytes.Buffer + tw := tar.NewWriter(&tarbuf) + for name, mode := range modes { + body := []byte("content") + if err := tw.WriteHeader(&tar.Header{Name: name, Typeflag: tar.TypeReg, Mode: mode, Size: int64(len(body))}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + var gzbuf bytes.Buffer + gz := gzip.NewWriter(&gzbuf) + if _, err := gz.Write(tarbuf.Bytes()); err != nil { + t.Fatal(err) + } + gz.Close() + + dir := t.TempDir() + if err := (&TarGzExtractor{}).Extract(&gzbuf, dir); err != nil { + t.Fatalf("Extract returned error: %v", err) + } + + for name := range modes { + info, err := os.Stat(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("stat %s: %v", name, err) + } + if info.Mode()&(os.ModeSetuid|os.ModeSetgid|os.ModeSticky) != 0 { + t.Errorf("%s: special bits not stripped: %v", name, info.Mode()) + } + if info.Mode().Perm()&0o022 != 0 { + t.Errorf("%s: group/other write not stripped: %#o", name, info.Mode().Perm()) + } + } + if info, _ := os.Stat(filepath.Join(dir, "setuid-binary")); info.Mode().Perm()&0o100 == 0 { + t.Errorf("owner execute bit not preserved: %#o", info.Mode().Perm()) + } +} + func TestCleanJoin(t *testing.T) { for i, fixture := range []struct { path string diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index 50d01522a..7759beff9 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -238,7 +238,7 @@ func extractTar(r io.Reader, targetDir string) error { return err } - outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, sanitizeArchiveMode(header.Mode)) if err != nil { return err }