Sanitize attacker-controlled file modes in plugin extraction

Signed-off-by: SABITHSAHEB <shabi7204192361@gmail.com>
pull/32249/head
SABITHSAHEB 4 months ago
parent be45213874
commit a2eb99d26c

@ -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
}

@ -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

@ -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
}

Loading…
Cancel
Save