diff --git a/container/files_test.go b/container/files_test.go index b222398c..3ed7da9d 100644 --- a/container/files_test.go +++ b/container/files_test.go @@ -141,9 +141,83 @@ 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) + }) + } +} + +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 +// 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 +244,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, header.FileInfo().Mode().Perm()) if err != nil { return err } // copy over contents if _, err := io.Copy(f, tr); err != nil { + f.Close() return err }