Merge pull request #105 from hashicorp/sec/os-root

sec: use os.Root for file manipulation

Author: CreatorHead

go.mod

@@ -1,18 +1,18 @@
 module github.com/hashicorp/go-slug
 
-go 1.24
+go 1.25
 
 require (
 	github.com/apparentlymart/go-versions v1.0.1
 	github.com/google/go-cmp v0.7.0
 	github.com/hashicorp/terraform-registry-address v0.4.0
 	github.com/hashicorp/terraform-svchost v0.1.1
-	golang.org/x/mod v0.26.0
-	golang.org/x/sys v0.35.0
+	golang.org/x/mod v0.28.0
+	golang.org/x/sys v0.36.0
 )
 
 require (
 	github.com/go-test/deep v1.0.3 // indirect
-	golang.org/x/net v0.43.0 // indirect
-	golang.org/x/text v0.28.0 // indirect
+	golang.org/x/net v0.44.0 // indirect
+	golang.org/x/text v0.29.0 // indirect
 )

go.sum

@@ -13,11 +13,11 @@ github.com/hashicorp/terraform-svchost v0.1.1 h1:EZZimZ1GxdqFRinZ1tpJwVxxt49xc/S
 github.com/hashicorp/terraform-svchost v0.1.1/go.mod h1:mNsjQfZyf/Jhz35v6/0LWcv26+X7JPS+buii2c9/ctc=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348 h1:MtvEpTB6LX3vkb4ax0b5D2DHbNAUsen0Gx5wZoq3lV4=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
-golang.org/x/mod v0.26.0 h1:EGMPT//Ezu+ylkCijjPc+f4Aih7sZvaAr+O3EHBxvZg=
-golang.org/x/mod v0.26.0/go.mod h1:/j6NAhSk8iQ723BGAUyoAcn7SlD7s15Dp9Nd/SfeaFQ=
-golang.org/x/net v0.43.0 h1:lat02VYK2j4aLzMzecihNvTlJNQUq316m2Mr9rnM6YE=
-golang.org/x/net v0.43.0/go.mod h1:vhO1fvI4dGsIjh73sWfUVjj3N7CA9WkKJNQm2svM6Jg=
-golang.org/x/sys v0.35.0 h1:vz1N37gP5bs89s7He8XuIYXpyY0+QlsKmzipCbUtyxI=
-golang.org/x/sys v0.35.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k=
-golang.org/x/text v0.28.0 h1:rhazDwis8INMIwQ4tpjLDzUhx6RlXqZNPEM0huQojng=
-golang.org/x/text v0.28.0/go.mod h1:U8nCwOR8jO/marOQ0QbDiOngZVEBB7MAiitBuMjXiNU=
+golang.org/x/mod v0.28.0 h1:gQBtGhjxykdjY9YhZpSlZIsbnaE2+PgjfLWUQTnoZ1U=
+golang.org/x/mod v0.28.0/go.mod h1:yfB/L0NOf/kmEbXjzCPOx1iK1fRutOydrCMsqRhEBxI=
+golang.org/x/net v0.44.0 h1:evd8IRDyfNBMBTTY5XRF1vaZlD+EmWx6x8PkhR04H/I=
+golang.org/x/net v0.44.0/go.mod h1:ECOoLqd5U3Lhyeyo/QDCEVQ4sNgYsqvCZ722XogGieY=
+golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k=
+golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
+golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk=
+golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4=

internal/ignorefiles/ignorerules.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path/filepath"
 )
 
 // A Ruleset is the result of reading, parsing, and compiling a
@@ -48,7 +47,14 @@ func ParseIgnoreFileContent(r io.Reader) (*Ruleset, error) {
 // This function will return an error only if an ignore file is present but
 // unreadable, or if an ignore file is present but contains invalid syntax.
 func LoadPackageIgnoreRules(packageDir string) (*Ruleset, error) {
-	file, err := os.Open(filepath.Join(packageDir, ".terraformignore"))
+	// Use os.Root for secure file access within the package directory
+	packageRoot, err := os.OpenRoot(packageDir)
+	if err != nil {
+		return nil, fmt.Errorf("cannot open package directory %q: %s", packageDir, err)
+	}
+	defer func() { _ = packageRoot.Close() }()
+
+	file, err := packageRoot.Open(".terraformignore")
 	if err != nil {
 		if os.IsNotExist(err) {
 			return DefaultRuleset, nil

internal/unpackinfo/unpackinfo.go

@@ -140,6 +140,28 @@ func (i UnpackInfo) RestoreInfo() error {
 	}
 }
 
+// RestoreInfoWithRoot changes the file mode and timestamps for the given UnpackInfo data
+// using os.Root for enhanced security to ensure operations stay within the destination
+func (i UnpackInfo) RestoreInfoWithRoot(root *os.Root, dst string) error {
+	// Calculate relative path from dst to i.Path for Root operations
+	relPath, err := filepath.Rel(dst, i.Path)
+	if err != nil {
+		return fmt.Errorf("failed to get relative path: %w", err)
+	}
+
+	switch {
+	case i.IsDirectory():
+		return i.restoreDirectoryWithRoot(root, relPath)
+	case i.IsSymlink():
+		if CanMaintainSymlinkTimestamps() {
+			return i.restoreSymlinkWithRoot(root, relPath)
+		}
+		return nil
+	default: // Normal file
+		return i.restoreNormalWithRoot(root, relPath)
+	}
+}
+
 func (i UnpackInfo) restoreDirectory() error {
 	if err := os.Chmod(i.Path, i.OriginalMode); err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("failed setting permissions on directory %q: %w", i.Path, err)
@@ -168,3 +190,34 @@ func (i UnpackInfo) restoreNormal() error {
 	}
 	return nil
 }
+
+func (i UnpackInfo) restoreDirectoryWithRoot(root *os.Root, relPath string) error {
+	if err := root.Chmod(relPath, i.OriginalMode); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed setting permissions on directory %q: %w", i.Path, err)
+	}
+
+	if err := root.Chtimes(relPath, i.OriginalAccessTime, i.OriginalModTime); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed setting times on directory %q: %w", i.Path, err)
+	}
+	return nil
+}
+
+func (i UnpackInfo) restoreSymlinkWithRoot(root *os.Root, relPath string) error {
+	// Note: Go 1.25's os.Root doesn't have Lchtimes, so we fall back to the original method
+	// This is a limitation but still provides some security benefit for other operations
+	if err := i.Lchtimes(); err != nil {
+		return fmt.Errorf("failed setting times on symlink %q: %w", i.Path, err)
+	}
+	return nil
+}
+
+func (i UnpackInfo) restoreNormalWithRoot(root *os.Root, relPath string) error {
+	if err := root.Chmod(relPath, i.OriginalMode); err != nil {
+		return fmt.Errorf("failed setting permissions on %q: %w", i.Path, err)
+	}
+
+	if err := root.Chtimes(relPath, i.OriginalAccessTime, i.OriginalModTime); err != nil {
+		return fmt.Errorf("failed setting times on %q: %w", i.Path, err)
+	}
+	return nil
+}

slug.go

@@ -390,6 +390,13 @@ func (p *Packer) Unpack(r io.Reader, dst string) error {
 	// for more details about how tar attempts to preserve file metadata.
 	directoriesExtracted := []unpackinfo.UnpackInfo{}
 
+	// Create a root for secure file operations - this prevents access outside dst
+	root, err := os.OpenRoot(dst)
+	if err != nil {
+		return fmt.Errorf("failed to open root directory %q: %w", dst, err)
+	}
+	defer func() { _ = root.Close() }()
+
 	// Decompress as we read.
 	uncompressed, err := gzip.NewReader(r)
 	if err != nil {
@@ -413,36 +420,37 @@ func (p *Packer) Unpack(r io.Reader, dst string) error {
 		if header.Name == "" {
 			continue
 		}
+		header.Name = filepath.Clean(header.Name)
 
 		info, err := unpackinfo.NewUnpackInfo(dst, header)
 		if err != nil {
 			return &IllegalSlugError{Err: err}
 		}
 
-		// Make the directories to the path.
-		dir := filepath.Dir(info.Path)
-
-		// Timestamps and permissions will be restored after all files are extracted.
-		if err := os.MkdirAll(dir, 0755); err != nil {
-			return fmt.Errorf("failed to create directory %q: %w", dir, err)
+		// Make the directories to the path using root for security.
+		dir := filepath.Dir(header.Name)
+		if dir != "." {
+			// Timestamps and permissions will be restored after all files are extracted.
+			if err := root.MkdirAll(dir, 0755); err != nil {
+				return fmt.Errorf("failed to create directory %q: %w", dir, err)
+			}
 		}
 
 		// Handle symlinks, directories, non-regular files
 		if info.IsSymlink() {
 
 			if ok, err := p.validSymlink(dst, header.Name, header.Linkname); ok {
-				// Create the symlink.
-				headerName := filepath.Clean(header.Name)
+				// Create the symlink using root for security.
 				headerLinkname := filepath.Clean(header.Linkname)
-				if err = os.Symlink(headerLinkname, info.Path); err != nil {
+				if err = root.Symlink(headerLinkname, header.Name); err != nil {
 					return fmt.Errorf("failed creating symlink (%q -> %q): %w",
-						headerName, headerLinkname, err)
+						header.Name, headerLinkname, err)
 				}
 			} else {
 				return err
 			}
 
-			if err := info.RestoreInfo(); err != nil {
+			if err := info.RestoreInfoWithRoot(root, dst); err != nil {
 				return err
 			}
 
@@ -461,16 +469,16 @@ func (p *Packer) Unpack(r io.Reader, dst string) error {
 			continue
 		}
 
-		// Open a handle to the destination.
-		fh, err := os.Create(info.Path)
+		// Open a handle to the destination using root for security.
+		fh, err := root.Create(header.Name)
 		if err != nil {
 			// This mimics tar's behavior wrt the tar file containing duplicate files
 			// and it allowing later ones to clobber earlier ones even if the file
 			// has perms that don't allow overwriting. The file permissions will be restored
 			// once the file contents are copied.
 			if os.IsPermission(err) {
-				_ = os.Chmod(info.Path, 0600)
-				fh, err = os.Create(info.Path)
+				_ = root.Chmod(header.Name, 0600)
+				fh, err = root.Create(header.Name)
 			}
 
 			if err != nil {
@@ -485,13 +493,13 @@ func (p *Packer) Unpack(r io.Reader, dst string) error {
 			return fmt.Errorf("failed to copy slug file %q: %w", info.Path, err)
 		}
 
-		if err := info.RestoreInfo(); err != nil {
+		if err := info.RestoreInfoWithRoot(root, dst); err != nil {
 			return err
 		}
 	}
 
 	for _, dir := range directoriesExtracted {
-		if err := dir.RestoreInfo(); err != nil {
+		if err := dir.RestoreInfoWithRoot(root, dst); err != nil {
 			return err
 		}
 	}