feat: cannot extract files outside the target directory

internal/deb/extract.go

@@ -213,7 +213,8 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
 				delete(tarDirMode, path)
 
 				createOptions := &fsutil.CreateOptions{
-					Path:        filepath.Join(options.TargetDir, path),
+					Root:        options.TargetDir,
+					Path:        path,
 					Mode:        mode,
 					MakeParents: true,
 				}
@@ -225,19 +226,13 @@ func extractData(pkgReader io.ReadSeeker, options *ExtractOptions) error {
 			link := tarHeader.Linkname
 			if tarHeader.Typeflag == tar.TypeLink {
 				// A hard link requires the real path of the target file.
-				// TODO This path must be sanitized to prevent a crafted
-				// tarball from referring to content outside the target
-				// directory. This issue also affects targetPath, but it's more
-				// relevant here as this is importing external content into the
-				// target directory. This is not a more serious issue because
-				// these tarballs are signed official packages, but must be
-				// fixed regardless.
 				link = filepath.Join(options.TargetDir, link)
 			}
 
 			// Create the entry itself.
 			createOptions := &fsutil.CreateOptions{
-				Path:         filepath.Join(options.TargetDir, targetPath),
+				Root:         options.TargetDir,
+				Path:         targetPath,
 				Mode:         tarHeader.FileInfo().Mode(),
 				Data:         pathReader,
 				Link:         link,
@@ -342,7 +337,8 @@ func extractHardLinks(pkgReader io.ReadSeeker, opts *extractHardLinkOptions) err
 		absLink := filepath.Join(opts.TargetDir, links[0].path)
 		// Extract the content to the first hard link path.
 		createOptions := &fsutil.CreateOptions{
-			Path: absLink,
+			Root: opts.TargetDir,
+			Path: links[0].path,
 			Mode: tarHeader.FileInfo().Mode(),
 			Data: tarReader,
 		}

internal/deb/extract_test.go

@@ -458,6 +458,34 @@ var extractTests = []extractTest{{
 		"/dir/": "dir 0777",
 	},
 	notCreated: []string{},
+}, {
+	summary: "Hardlink cannot escape target directory",
+	pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
+		testutil.Dir(0755, "./"),
+		testutil.Hrd(0644, "./hardlink", "/etc/group"),
+	}),
+	options: deb.ExtractOptions{
+		Extract: map[string][]deb.ExtractInfo{
+			"/**": []deb.ExtractInfo{{
+				Path: "/**",
+			}},
+		},
+	},
+	error: `cannot extract from package "test-package": invalid link target /etc/group`,
+}, {
+	summary: "Cannot extract outside of target directory",
+	pkgdata: testutil.MustMakeDeb([]testutil.TarEntry{
+		testutil.Dir(0755, "./"),
+		testutil.Reg(0644, "./../file", "hijacking system file"),
+	}),
+	options: deb.ExtractOptions{
+		Extract: map[string][]deb.ExtractInfo{
+			"/**": []deb.ExtractInfo{{
+				Path: "/**",
+			}},
+		},
+	},
+	error: `cannot extract from package "test-package": cannot create path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`,
 }}
 
 func (s *S) TestExtract(c *C) {

internal/fsutil/create.go

@@ -9,9 +9,13 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
 )
 
 type CreateOptions struct {
+	// Root defaults to "/" if empty.
+	Root string
+	// Path is relative to Root.
 	Path string
 	Mode fs.FileMode
 	Data io.Reader
@@ -45,6 +49,17 @@ func Create(options *CreateOptions) (*Entry, error) {
 	optsCopy.Data = rp
 	o := &optsCopy
 
+	if o.Root == "" {
+		o.Root = "/"
+	}
+	if o.Root != "/" {
+		o.Root = filepath.Clean(o.Root) + "/"
+	}
+	o.Path = filepath.Clean(filepath.Join(o.Root, o.Path))
+	if !strings.HasPrefix(o.Path, o.Root) {
+		return nil, fmt.Errorf("cannot create path %s outside of root %s", o.Path, o.Root)
+	}
+
 	var err error
 	var hash string
 	if o.MakeParents {
@@ -56,6 +71,12 @@ func Create(options *CreateOptions) (*Entry, error) {
 	switch o.Mode & fs.ModeType {
 	case 0:
 		if o.Link != "" {
+			o.Link = filepath.Clean(o.Link)
+			if filepath.IsAbs(o.Link) {
+				if !strings.HasPrefix(o.Link, o.Root) {
+					return nil, fmt.Errorf("invalid hardlink %s target: %s is outside of root %s", o.Path, o.Link, o.Root)
+				}
+			}
 			err = createHardLink(o)
 		} else {
 			err = createFile(o)
@@ -108,21 +129,35 @@ func Create(options *CreateOptions) (*Entry, error) {
 // information recorded in Entry. The Hash and Size attributes are set on
 // calling Close() on the Writer.
 func CreateWriter(options *CreateOptions) (io.WriteCloser, *Entry, error) {
-	if !options.Mode.IsRegular() {
-		return nil, nil, fmt.Errorf("unsupported file type: %s", options.Path)
+	optsCopy := *options
+	o := &optsCopy
+
+	if o.Root == "" {
+		o.Root = "/"
 	}
-	if options.MakeParents {
-		if err := os.MkdirAll(filepath.Dir(options.Path), 0755); err != nil {
+	if o.Root != "/" {
+		o.Root = filepath.Clean(o.Root) + "/"
+	}
+	o.Path = filepath.Clean(filepath.Join(o.Root, o.Path))
+	if !strings.HasPrefix(o.Path, o.Root) {
+		return nil, nil, fmt.Errorf("cannot create path %s outside of root %s", o.Path, o.Root)
+	}
+	if !o.Mode.IsRegular() {
+		return nil, nil, fmt.Errorf("unsupported file type: %s", o.Path)
+	}
+	if o.MakeParents {
+		if err := os.MkdirAll(filepath.Dir(o.Path), 0755); err != nil {
 			return nil, nil, err
 		}
 	}
-	file, err := os.OpenFile(options.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, options.Mode)
+
+	file, err := os.OpenFile(o.Path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, o.Mode)
 	if err != nil {
 		return nil, nil, err
 	}
 	entry := &Entry{
-		Path: options.Path,
-		Mode: options.Mode,
+		Path: o.Path,
+		Mode: o.Mode,
 	}
 	wp := &writerProxy{
 		entry: entry,

internal/fsutil/create_test.go

@@ -271,6 +271,33 @@ var createTests = []createTest{{
 	result: map[string]string{
 		"/bar": "symlink foo",
 	},
+}, {
+	summary: "Cannot create file outside of Root",
+	options: fsutil.CreateOptions{
+		Root: "/root",
+		Path: "../file",
+		Mode: 0666,
+		Data: bytes.NewBufferString("hijacking system file"),
+	},
+	error: `cannot create path /file outside of root /root/`,
+}, {
+	summary: "Hardlink cannot escape Root",
+	options: fsutil.CreateOptions{
+		Root: "/root",
+		Path: "/system-file",
+		Link: "/etc/group",
+		Mode: 0666,
+	},
+	error: `invalid hardlink /root/system-file target: /etc/group is outside of root /root/`,
+}, {
+	summary: "Root is correctly used as prefix",
+	options: fsutil.CreateOptions{
+		Root: "/foo",
+		Path: "/system-file",
+		Link: "/foobar",
+		Mode: 0666,
+	},
+	error: `invalid hardlink /foo/system-file target: /foobar is outside of root /foo/`,
 }}
 
 func (s *S) TestCreate(c *C) {
@@ -288,7 +315,9 @@ func (s *S) TestCreate(c *C) {
 		c.Logf("Options: %v", test.options)
 		dir := c.MkDir()
 		options := test.options
-		options.Path = filepath.Join(dir, options.Path)
+		if options.Root == "" {
+			options.Root = dir
+		}
 		if test.hackopt != nil {
 			test.hackopt(c, dir, &options)
 		}
@@ -374,6 +403,14 @@ var createWriterTests = []createWriterTest{{
 		MakeParents: false,
 	},
 	error: `open /[a-z0-9\-\/]*/foo/bar: no such file or directory`,
+}, {
+	options: fsutil.CreateOptions{
+		Root:        "/root",
+		Path:        "../file",
+		Mode:        0644,
+		MakeParents: true,
+	},
+	error: `cannot create path /file outside of root /root/`,
 }}
 
 func (s *S) TestCreateWriter(c *C) {
@@ -393,7 +430,9 @@ func (s *S) TestCreateWriter(c *C) {
 			test.hackdir(c, dir)
 		}
 		options := test.options
-		options.Path = filepath.Join(dir, options.Path)
+		if test.options.Root == "" {
+			options.Root = dir
+		}
 		writer, entry, err := fsutil.CreateWriter(&options)
 		if test.error != "" {
 			c.Assert(err, ErrorMatches, test.error)
@@ -404,7 +443,7 @@ func (s *S) TestCreateWriter(c *C) {
 		// Hash and Size are only set when the writer is closed.
 		_, err = writer.Write(test.data)
 		c.Assert(err, IsNil)
-		c.Assert(entry.Path, Equals, options.Path)
+		c.Assert(entry.Path, Equals, filepath.Clean(filepath.Join(options.Root, options.Path)))
 		c.Assert(entry.Mode, Equals, options.Mode)
 		c.Assert(entry.SHA256, Equals, "")
 		c.Assert(entry.Size, Equals, 0)

internal/slicer/slicer_test.go

@@ -1743,6 +1743,46 @@ var slicerTests = []slicerTest{{
 		"/file":     "file 0644 2c26b46b <1> {test-package_myslice}",
 		"/hardlink": "file 0644 2c26b46b <1> {test-package_myslice}",
 	},
+}, {
+	summary: "Hard links cannot escape the target directory",
+	slices:  []setup.SliceKey{{"test-package", "myslice"}},
+	pkgs: []*testutil.TestPackage{{
+		Name: "test-package",
+		Data: testutil.MustMakeDeb([]testutil.TarEntry{
+			testutil.Dir(0755, "./"),
+			testutil.Hrd(0644, "./hardlink", "/etc/group"),
+		}),
+	}},
+	release: map[string]string{
+		"slices/mydir/test-package.yaml": `
+			package: test-package
+			slices:
+				myslice:
+					contents:
+						/hardlink:
+		`,
+	},
+	error: `cannot extract from package "test-package": invalid link target /etc/group`,
+}, {
+	summary: "Cannot extract outside of target directory",
+	slices:  []setup.SliceKey{{"test-package", "myslice"}},
+	pkgs: []*testutil.TestPackage{{
+		Name: "test-package",
+		Data: testutil.MustMakeDeb([]testutil.TarEntry{
+			testutil.Dir(0755, "./"),
+			testutil.Reg(0644, "./../file", "hijacking system file"),
+		}),
+	}},
+	release: map[string]string{
+		"slices/mydir/test-package.yaml": `
+			package: test-package
+			slices:
+				myslice:
+					contents:
+						/**:
+		`,
+	},
+	error: `cannot extract from package "test-package": cannot create path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`,
 }}
 
 var defaultChiselYaml = `