Constrain tar driver paths to image root

internal/pkgutil/root_path.go

@@ -0,0 +1,92 @@
+package pkgutil
+
+import (
+	"fmt"
+	"os"
+	pathpkg "path"
+	"path/filepath"
+	"strings"
+)
+
+const maxRootPathSymlinks = 255
+
+// ResolvePathInRoot resolves imagePath below root. Absolute paths are treated as
+// absolute within root, and symlink targets are resolved using the same image
+// root instead of the host root.
+func ResolvePathInRoot(root, imagePath string, followFinalSymlink bool) (string, error) {
+	absRoot, err := filepath.Abs(root)
+	if err != nil {
+		return "", err
+	}
+	absRoot = filepath.Clean(absRoot)
+
+	parts, err := resolveRootPathParts(nil, imagePath)
+	if err != nil {
+		return "", err
+	}
+
+	resolved := []string{}
+	symlinkCount := 0
+	for i := 0; i < len(parts); i++ {
+		part := parts[i]
+		candidate := joinRootPath(absRoot, append(resolved, part))
+		isFinal := i == len(parts)-1
+		if !isFinal || followFinalSymlink {
+			info, err := os.Lstat(candidate)
+			if err != nil && !os.IsNotExist(err) {
+				return "", err
+			}
+			if err == nil && info.Mode()&os.ModeSymlink != 0 {
+				symlinkCount++
+				if symlinkCount > maxRootPathSymlinks {
+					return "", fmt.Errorf("too many symlinks resolving path %q", imagePath)
+				}
+				linkTarget, err := os.Readlink(candidate)
+				if err != nil {
+					return "", err
+				}
+				base := resolved
+				if pathpkg.IsAbs(filepath.ToSlash(linkTarget)) {
+					base = nil
+				}
+				linkParts, err := resolveRootPathParts(base, linkTarget)
+				if err != nil {
+					return "", err
+				}
+				parts = append(linkParts, parts[i+1:]...)
+				resolved = nil
+				i = -1
+				continue
+			}
+		}
+		resolved = append(resolved, part)
+	}
+	return joinRootPath(absRoot, resolved), nil
+}
+
+func resolveRootPathParts(base []string, imagePath string) ([]string, error) {
+	cleaned := filepath.ToSlash(imagePath)
+	if pathpkg.IsAbs(cleaned) {
+		base = nil
+	}
+
+	parts := append([]string{}, base...)
+	for _, part := range strings.Split(pathpkg.Clean(cleaned), "/") {
+		switch part {
+		case "", ".":
+			continue
+		case "..":
+			if len(parts) == 0 {
+				return nil, fmt.Errorf("path %q escapes root", imagePath)
+			}
+			parts = parts[:len(parts)-1]
+		default:
+			parts = append(parts, part)
+		}
+	}
+	return parts, nil
+}
+
+func joinRootPath(root string, parts []string) string {
+	return filepath.Join(append([]string{root}, parts...)...)
+}

internal/pkgutil/tar_utils.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	pathpkg "path"
 	"path/filepath"
 	"strings"
 
@@ -50,7 +51,10 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 		if err != nil {
 			return errors.Wrap(err, "Error getting next tar header")
 		}
-		target := filepath.Clean(filepath.Join(path, header.Name))
+		target, err := ResolvePathInRoot(path, header.Name, false)
+		if err != nil {
+			return errors.Wrap(err, "resolving tar entry path")
+		}
 		// Make sure the target isn't part of the whitelist
 		if checkWhitelist(target, whitelist) {
 			continue
@@ -60,7 +64,7 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 
 		// if its a dir and it doesn't exist create it
 		case tar.TypeDir:
-			if _, err := os.Stat(target); os.IsNotExist(err) {
+			if _, err := os.Lstat(target); os.IsNotExist(err) {
 				if mode.Perm()&(1<<(uint(7))) == 0 {
 					logrus.Debugf("Write permission bit not set on %s by default; setting manually", target)
 					originalMode := mode
@@ -93,7 +97,7 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 			}
 			// It's possible we end up creating files that can't be overwritten based on their permissions.
 			// Explicitly delete an existing file before continuing.
-			if _, err := os.Stat(target); !os.IsNotExist(err) {
+			if _, err := os.Lstat(target); !os.IsNotExist(err) {
 				logrus.Debugf("Removing %s for overwrite", target)
 				if err := os.Remove(target); err != nil {
 					logrus.Errorf("error removing file %s", target)
@@ -118,9 +122,12 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 			}
 			currFile.Close()
 		case tar.TypeSymlink:
+			if _, err := ResolvePathInRoot(path, tarLinkTarget(header.Name, header.Linkname), true); err != nil {
+				return errors.Wrap(err, "resolving tar symlink path")
+			}
 			// It's possible we end up creating files that can't be overwritten based on their permissions.
 			// Explicitly delete an existing file before continuing.
-			if _, err := os.Stat(target); !os.IsNotExist(err) {
+			if _, err := os.Lstat(target); !os.IsNotExist(err) {
 				logrus.Debugf("Removing %s to create symlink", target)
 				if err := os.RemoveAll(target); err != nil {
 					logrus.Debugf("Unable to remove %s: %s", target, err)
@@ -131,11 +138,16 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 				logrus.Errorf("Failed to create symlink between %s and %s: %s", header.Linkname, target, err)
 			}
 		case tar.TypeLink:
-			linkname := filepath.Clean(filepath.Join(path, header.Linkname))
+			linkname, err := ResolvePathInRoot(path, header.Linkname, false)
+			if err != nil {
+				return errors.Wrap(err, "resolving tar hard link path")
+			}
 			// Check if the linkname already exists
 			if _, err := os.Stat(linkname); !os.IsNotExist(err) {
 				// If it exists, create the hard link
-				resolveHardlink(linkname, target)
+				if err := resolveHardlink(path, linkname, target); err != nil {
+					return errors.Wrap(err, fmt.Sprintf("Unable to create hard link from %s to %s", linkname, target))
+				}
 			} else {
 				hardlinks.Store(target, linkname)
 			}
@@ -148,7 +160,7 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 		logrus.Info("Resolving hard links")
 		if _, err := os.Stat(linkname); !os.IsNotExist(err) {
 			// If it exists, create the hard link
-			if err := resolveHardlink(linkname, target); err != nil {
+			if err := resolveHardlink(path, linkname, target); err != nil {
 				resolveError.Store(errors.Wrap(err, fmt.Sprintf("Unable to create hard link from %s to %s", linkname, target)))
 				return false
 			}
@@ -168,7 +180,36 @@ func unpackTar(tr *tar.Reader, path string, whitelist []string) error {
 	return nil
 }
 
-func resolveHardlink(linkname, target string) error {
+func tarLinkTarget(name, linkname string) string {
+	if pathpkg.IsAbs(filepath.ToSlash(linkname)) {
+		return linkname
+	}
+	return pathpkg.Join(pathpkg.Dir(filepath.ToSlash(name)), filepath.ToSlash(linkname))
+}
+
+func resolveHardlink(root, linkname, target string) error {
+	info, err := os.Lstat(linkname)
+	if err != nil {
+		return err
+	}
+	if info.Mode()&os.ModeSymlink != 0 {
+		linkTarget, err := os.Readlink(linkname)
+		if err != nil {
+			return err
+		}
+		relLink, err := filepath.Rel(root, linkname)
+		if err != nil {
+			return err
+		}
+		if _, err := ResolvePathInRoot(root, tarLinkTarget(relLink, linkTarget), true); err != nil {
+			return err
+		}
+		if err := os.Symlink(linkTarget, target); err != nil {
+			return err
+		}
+		logrus.Debugf("Created symlink from %s to %s for hard link source %s", linkTarget, target, linkname)
+		return nil
+	}
 	if err := os.Link(linkname, target); err != nil {
 		return err
 	}

internal/pkgutil/tar_utils_test.go

@@ -0,0 +1,178 @@
+package pkgutil
+
+import (
+	"archive/tar"
+	"bytes"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func testTarReader(t *testing.T, entries ...func(*tar.Writer) error) *tar.Reader {
+	t.Helper()
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	for _, entry := range entries {
+		if err := entry(tw); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return tar.NewReader(bytes.NewReader(buf.Bytes()))
+}
+
+func testTarFile(name, body string) func(*tar.Writer) error {
+	return func(tw *tar.Writer) error {
+		if err := tw.WriteHeader(&tar.Header{
+			Name: name,
+			Mode: 0644,
+			Size: int64(len(body)),
+		}); err != nil {
+			return err
+		}
+		_, err := tw.Write([]byte(body))
+		return err
+	}
+}
+
+func testTarDir(name string) func(*tar.Writer) error {
+	return func(tw *tar.Writer) error {
+		return tw.WriteHeader(&tar.Header{
+			Name:     name,
+			Mode:     0755,
+			Typeflag: tar.TypeDir,
+		})
+	}
+}
+
+func testTarSymlink(name, linkname string) func(*tar.Writer) error {
+	return func(tw *tar.Writer) error {
+		return tw.WriteHeader(&tar.Header{
+			Name:     name,
+			Linkname: linkname,
+			Typeflag: tar.TypeSymlink,
+		})
+	}
+}
+
+func testTarHardlink(name, linkname string) func(*tar.Writer) error {
+	return func(tw *tar.Writer) error {
+		return tw.WriteHeader(&tar.Header{
+			Name:     name,
+			Linkname: linkname,
+			Typeflag: tar.TypeLink,
+		})
+	}
+}
+
+func TestUnpackTarRejectsEntriesOutsideRoot(t *testing.T) {
+	parent := t.TempDir()
+	root := filepath.Join(parent, "root")
+	if err := os.MkdirAll(root, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	err := unpackTar(testTarReader(t, testTarFile("../outside.txt", "outside-data")), root, nil)
+	if err == nil {
+		t.Fatal("unpackTar allowed an entry outside the root")
+	}
+	if _, err := os.Stat(filepath.Join(parent, "outside.txt")); !os.IsNotExist(err) {
+		t.Fatalf("outside path exists after rejected unpack: %v", err)
+	}
+}
+
+func TestUnpackTarRejectsSymlinksOutsideRoot(t *testing.T) {
+	parent := t.TempDir()
+	root := filepath.Join(parent, "root")
+	if err := os.MkdirAll(root, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	err := unpackTar(testTarReader(t, testTarSymlink("link", "..")), root, nil)
+	if err == nil {
+		t.Fatal("unpackTar allowed a symlink outside the root")
+	}
+}
+
+func TestUnpackTarRejectsHardlinksOutsideRoot(t *testing.T) {
+	root := t.TempDir()
+
+	err := unpackTar(testTarReader(t, testTarHardlink("link", "../outside.txt")), root, nil)
+	if err == nil {
+		t.Fatal("unpackTar allowed a hard link outside the root")
+	}
+}
+
+func TestUnpackTarHardlinkToSymlinkDoesNotFollowTarget(t *testing.T) {
+	root := t.TempDir()
+
+	err := unpackTar(testTarReader(t,
+		testTarFile("file.txt", "image-data"),
+		testTarSymlink("sym", "file.txt"),
+		testTarHardlink("hard", "sym"),
+	), root, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	info, err := os.Lstat(filepath.Join(root, "hard"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if info.Mode()&os.ModeSymlink == 0 {
+		t.Fatalf("hard link to symlink should remain a symlink, got mode %s", info.Mode())
+	}
+	target, err := os.Readlink(filepath.Join(root, "hard"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if target != "file.txt" {
+		t.Fatalf("hard link to symlink target = %q", target)
+	}
+}
+
+func TestUnpackTarRejectsHardlinkSourceSymlinkOutsideRoot(t *testing.T) {
+	parent := t.TempDir()
+	root := filepath.Join(parent, "root")
+	if err := os.MkdirAll(root, 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(parent, "outside.txt"), []byte("outside-data"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("../outside.txt", filepath.Join(root, "sym")); err != nil {
+		t.Fatal(err)
+	}
+
+	err := unpackTar(testTarReader(t, testTarHardlink("hard", "sym")), root, nil)
+	if err == nil {
+		t.Fatal("unpackTar allowed a hard link through a symlink outside the root")
+	}
+	if _, err := os.Lstat(filepath.Join(root, "hard")); !os.IsNotExist(err) {
+		t.Fatalf("hard link path exists after rejected unpack: %v", err)
+	}
+}
+
+func TestUnpackTarResolvesSymlinksWithinRoot(t *testing.T) {
+	root := t.TempDir()
+
+	err := unpackTar(testTarReader(t,
+		testTarDir("etc"),
+		testTarSymlink("link", "/etc"),
+		testTarFile("link/generated.txt", "image-data"),
+	), root, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := os.ReadFile(filepath.Join(root, "etc", "generated.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(got) != "image-data" {
+		t.Fatalf("unpacked file through image symlink = %q", got)
+	}
+}