From 9241b633a464e9c0b71c7cb017400e843e589324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Mon, 26 Oct 2026 21:20:53 +0100 Subject: [PATCH 1/2] fix(container): prevent path traversal in untar test helper Scope all filesystem writes in the untar test helper to an os.Root on the destination directory, so archive entries escaping it are rejected. Co-Authored-By: Claude Opus 5.5 (1M context) --- container/files_test.go | 77 +++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 15 deletions(-) diff --git a/container/files_test.go b/container/files_test.go index b222398c..9e430487 100644 --- a/container/files_test.go +++ b/container/files_test.go @@ -141,9 +141,64 @@ func TestTarFile(t *testing.T) { require.Equal(t, b, untarBytes) } +func TestUntar_rejectsPathTraversal(t *testing.T) { + tests := []struct { + name string + typeflag byte + }{ + {name: "../evil.txt", typeflag: tar.TypeReg}, + {name: "nested/../../evil.txt", typeflag: tar.TypeReg}, + {name: "/evil.txt", typeflag: tar.TypeReg}, + {name: "../evil-dir", typeflag: tar.TypeDir}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + content := []byte("evil") + + var buf bytes.Buffer + gw := gzip.NewWriter(&buf) + tw := tar.NewWriter(gw) + hdr := &tar.Header{Name: tc.name, Typeflag: tc.typeflag, Mode: 0o644} + if tc.typeflag == tar.TypeReg { + hdr.Size = int64(len(content)) + } + require.NoError(t, tw.WriteHeader(hdr)) + if tc.typeflag == tar.TypeReg { + _, err := tw.Write(content) + require.NoError(t, err) + } + require.NoError(t, tw.Close()) + require.NoError(t, gw.Close()) + + parent := t.TempDir() + dst := filepath.Join(parent, "dst") + require.NoError(t, os.Mkdir(dst, 0o755)) + + err := untar(dst, &buf) + require.Error(t, err) + + _, err = os.Stat(filepath.Join(parent, filepath.Base(tc.name))) + require.ErrorIs(t, err, os.ErrNotExist) + }) + } +} + // untar takes a destination path and a reader; a tar reader loops over the tarfile -// creating the file structure at 'dst' along the way, and writing any files +// creating the file structure at 'dst' along the way, and writing any files. +// All filesystem operations go through an os.Root scoped to 'dst', so entries +// whose names escape it (e.g. "../evil" or absolute paths) are rejected. func untar(dst string, r io.Reader) error { + if err := os.MkdirAll(dst, 0o755); err != nil { + return err + } + + root, err := os.OpenRoot(dst) + if err != nil { + return err + } + defer root.Close() + gzr, err := gzip.NewReader(r) if err != nil { return err @@ -170,33 +225,25 @@ func untar(dst string, r io.Reader) error { continue } - // the target location where the dir/file should be created - target := filepath.Join(dst, header.Name) - - // the following switch could also be done using fi.Mode(), not sure if there - // a benefit of using one vs. the other. - // fi := header.FileInfo() - // check the file type switch header.Typeflag { - // if it's a dir and it doesn't exist create it + // if it's a dir, create it (and any missing parents) inside the root case tar.TypeDir: - if _, err := os.Stat(target); err != nil { - if err := os.MkdirAll(target, 0o755); err != nil { - return err - } + if err := root.MkdirAll(header.Name, 0o755); err != nil { + return err } - // if it's a file create it + // if it's a file create it inside the root case tar.TypeReg: - f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + f, err := root.OpenFile(header.Name, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) if err != nil { return err } // copy over contents if _, err := io.Copy(f, tr); err != nil { + f.Close() return err } From 9c88745cf7ae6f2efeaa6d3ebeab87170c466bb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20de=20la=20Pe=C3=B1a?= Date: Tue, 27 Oct 2026 07:26:00 +0100 Subject: [PATCH 2/2] fix(container): mask special mode bits in untar test helper os.Root.OpenFile rejects permission bits outside 0o777, so archives with setuid, setgid or sticky bits could no longer be extracted. Co-Authored-By: Claude Opus 5.5 (1M context) --- container/files_test.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/container/files_test.go b/container/files_test.go index 9e430487..3ed7da9d 100644 --- a/container/files_test.go +++ b/container/files_test.go @@ -184,6 +184,25 @@ func TestUntar_rejectsPathTraversal(t *testing.T) { } } +func TestUntar_specialModeBits(t *testing.T) { + b := []byte("content") + + // os.Root rejects permission bits outside 0o777, so setuid/setgid/sticky + // bits must be masked before extracting. + buff, err := tarFile("special.file", func(tw io.Writer) error { + _, err := tw.Write(b) + return err + }, int64(len(b)), 0o4755) + require.NoError(t, err) + + tmpDir := t.TempDir() + require.NoError(t, untar(tmpDir, bytes.NewReader(buff.Bytes()))) + + untarBytes, err := os.ReadFile(filepath.Join(tmpDir, "special.file")) + require.NoError(t, err) + require.Equal(t, b, untarBytes) +} + // untar takes a destination path and a reader; a tar reader loops over the tarfile // creating the file structure at 'dst' along the way, and writing any files. // All filesystem operations go through an os.Root scoped to 'dst', so entries @@ -236,7 +255,7 @@ func untar(dst string, r io.Reader) error { // if it's a file create it inside the root case tar.TypeReg: - f, err := root.OpenFile(header.Name, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + f, err := root.OpenFile(header.Name, os.O_CREATE|os.O_RDWR, header.FileInfo().Mode().Perm()) if err != nil { return err }