CVE-2024-24579: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: stereoscope is a go library for processing container images and simulating a squash filesystem.  Prior to version 0.0.1, it is possible to craft an OCI tar archive that, when stereoscope attempts to unarchive the contents, will result in writing to paths outside of the unarchive temporary directory. Specifically, use of `github.com/anchore/stereoscope/pkg/file.UntarToDirectory()` function, the  `github.com/anchore/stereoscope/pkg/image/oci.TarballImageProvider` struct, or the higher level `github.com/anchore/stereoscope/pkg/image.Image.Read()` function express this vulnerability. As a workaround, if you are using the OCI archive as input into stereoscope then you can switch to using an OCI layout by unarchiving the tar archive and provide the unarchived directory to stereoscope.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 9.8
Published: 2024-01-31T17:15Z
Repository: https://github.com/anchore/stereoscope
Language: Go
Files changed: pkg/file/tarutil.go, pkg/file/tarutil_test.go
Commit date: 2024-01-31 10:43:03 -0500
Commit message: fix tar path traversal issue (#214)

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Diff: diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go
index 374c60f..2b3ed21 100644
--- a/pkg/file/tarutil.go
+++ b/pkg/file/tarutil.go
@@ -4,12 +4,14 @@ import (
 	"archive/tar"
 	"fmt"
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
+	"github.com/spf13/afero"
 
 	"github.com/anchore/stereoscope/internal/log"
 )
 
 const perFileReadLimit = 2 * GB
@@ -122,42 +124,63 @@ func MetadataFromTar(reader io.ReadCloser, tarPath string) (Metadata, error) {
 		return Metadata{}, &ErrFileNotFound{tarPath}
 	}
 	return *metadata, nil
 }
 
-// UntarToDirectory writes the contents of the given tar reader to the given destination
+// UntarToDirectory writes the contents of the given tar reader to the given destination. Note: this is meant to handle
+// archives for images (not image contents) thus intentionally does not handle links or any kinds of special files.
 func UntarToDirectory(reader io.Reader, dst string) error {
-	visitor := func(entry TarFileEntry) error {
-		target := filepath.Join(dst, entry.Header.Name)
-
-		switch entry.Header.Typeflag {
-		case tar.TypeDir:
-			if _, err := os.Stat(target); err != nil {
-				if err := os.MkdirAll(target, 0755); err != nil {
-					return err
-				}
-			}
+	return IterateTar(
+		reader,
+		tarVisitor{
+			fs:          afero.NewOsFs(),
+			destination: dst,
+		}.visit,
+	)
+}
+
+type tarVisitor struct {
+	fs          afero.Fs
+	destination string
+}
+
+func (v tarVisitor) visit(entry TarFileEntry) error {
+	target := filepath.Join(v.destination, entry.Header.Name)
 
-		case tar.TypeReg:
-			f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(entry.Header.Mode))
-			if err != nil {
+	// we should not allow for any destination path to be outside of where we are unarchiving to
+	if !strings.HasPrefix(target, v.destination) {
+		return fmt.Errorf("potential path traversal attack with entry: %q", entry.Header.Name)
+	}
+
+	switch entry.Header.Typeflag {
+	case tar.TypeSymlink, tar.TypeLink:
+		// we don't handle this is to prevent any potential traversal attacks
+		log.WithFields("path", entry.Header.Name).Trace("skipping symlink/link entry in image tar")
+
+	case tar.TypeDir:
+		if _, err := v.fs.Stat(target); err != nil {
+			if err := v.fs.MkdirAll(target, 0755); err != nil {
 				return err
 			}
+		}
 
-			// limit the reader on each file read to prevent decompression bomb attacks
-			numBytes, err := io.Copy(f, io.LimitReader(entry.Reader, perFileReadLimit))
-			if numBytes >= perFileReadLimit || errors.Is(err, io.EOF) {
-				return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
-			}
-			if err != nil {
-				return fmt.Errorf("unable to copy file: %w", err)
-			}
+	case tar.TypeReg:
+		f, err := v.fs.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(entry.Header.Mode))
+		if err != nil {
+			return err
+		}
 
-			if err = f.Close(); err != nil {
-				log.Errorf("failed to close file during untar of path=%q: %w", f.Name(), err)
-			}
+		// limit the reader on each file read to prevent decompression bomb attacks
+		numBytes, err := io.Copy(f, io.LimitReader(entry.Reader, perFileReadLimit))
+		if numBytes >= perFileReadLimit || errors.Is(err, io.EOF) {
+			return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
+		}
+		if err != nil {
+			return fmt.Errorf("unable to copy file: %w", err)
 		}
-		return nil
-	}
 
-	return IterateTar(reader, visitor)
+		if err = f.Close(); err != nil {
+			log.Errorf("failed to close file during untar of path=%q: %w", f.Name(), err)
+		}
+	}
+	return nil
 }
diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go
index 036c0c5..9e6032a 100644
--- a/pkg/file/tarutil_test.go
+++ b/pkg/file/tarutil_test.go
@@ -2,21 +2,28 @@
 // +build !windows
 
 package file
 
 import (
+	"archive/tar"
 	"crypto/sha256"
 	"fmt"
 	"io"
 	"os"
 	"os/exec"
 	"path"
 	"path/filepath"
+	"sort"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/scylladb/go-set/strset"
+	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 const (
 	fixturesPath = "test-fixtures"
 )
@@ -179,5 +186,219 @@ func fileExists(t testing.TB, filename string) bool {
 	} else if err != nil {
 		t.Fatal(err)
 	}
 	return !info.IsDir()
 }
+
+func Test_tarVisitor_visit(t *testing.T) {
+	assertNoFilesInRoot := func(t testing.TB, fs afero.Fs) {
+		t.Helper()
+
+		allowableFiles := strset.New("tmp")
+
+		// list all files in root
+		files, err := afero.ReadDir(fs, "/")
+		require.NoError(t, err)
+
+		for _, f := range files {
+			assert.True(t, allowableFiles.Has(f.Name()), "unexpected file in root: %s", f.Name())
+		}
+	}
+
+	assertPaths := func(expectedFiles []string, expectedDirs []string) func(t testing.TB, fs afero.Fs) {
+		return func(t testing.TB, fs afero.Fs) {
+			t.Helper()
+
+			sort.Strings(expectedFiles)
+			haveFiles := strset.New()
+			haveDirs := strset.New()
+			err := afero.Walk(fs, "/", func(path string, info os.FileInfo, err error) error {
+				require.NoError(t, err)
+				if info.IsDir() {
+					haveDirs.Add(path)
+				} else {
+					haveFiles.Add(path)
+				}
+				return nil
+			})
+
+			haveFilesList := haveFiles.List()
+			sort.Strings(haveFilesList)
+
+			haveDirsList := haveDirs.List()
+			sort.Strings(haveDirsList)
+
+			require.NoError(t, err)
+
+			if d := cmp.Diff(expectedFiles, haveFilesList); d != "" {
+				t.Errorf("unexpected files (-want +got):\n%s", d)
+			}
+
+			if d := cmp.Diff(expectedDirs, haveDirsList); d != "" {
+				t.Errorf("unexpected dirs (-want +got):\n%s", d)
+			}
+
+		}
+	}
+
+	tests := []struct {
+		name     string
+		entry    TarFileEntry
+		wantErr  require.ErrorAssertionFunc
+		assertFs []func(t testing.TB, fs afero.Fs)
+	}{
+		{
+			name: "regular file is written",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeReg,
+					Name:     "file.txt",
+					Linkname: "",
+					Size:     2,
+				},
+				Reader: strings.NewReader("hi"),
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{"/tmp/file.txt"},
+					[]string{"/", "/tmp"},
+				),
+			},
+		},
+		{
+			name: "regular file with possible path traversal errors out",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeReg,
+					Name:     "../file.txt",
+					Linkname: "",
+					Size:     2,
+				},
+				Reader: strings.NewReader("hi"),
+			},
+			wantErr: require.Error,
+		},
+		{
+			name: "directory is created",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeDir,
+					Name:     "dir",
+					Linkname: "",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/", "/tmp", "/tmp/dir"},
+				),
+			},
+		},
+		{
+			name: "symlink is ignored",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeSymlink,
+					Name:     "symlink",
+					Linkname: "./../to-location",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/"},
+				),
+			},
+		},
+		{
+			name: "hardlink is ignored",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeLink,
+					Name:     "link",
+					Linkname: "./../to-location",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/"},
+				),
+			},
+		},
+		{
+			name: "device is ignored",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeChar,
+					Name:     "device",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/"},
+				),
+			},
+		},
+		{
+			name: "block device is ignored",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeBlock,
+					Name:     "device",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/"},
+				),
+			},
+		},
+		{
+			name: "pipe is ignored",
+			entry: TarFileEntry{
+				Sequence: 0,
+				Header: tar.Header{
+					Typeflag: tar.TypeFifo,
+					Name:     "pipe",
+				},
+			},
+			assertFs: []func(t testing.TB, fs afero.Fs){
+				assertPaths(
+					[]string{},
+					[]string{"/"},
+				),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.wantErr == nil {
+				tt.wantErr = require.NoError
+			}
+			v := tarVisitor{
+				fs:          afero.NewMemMapFs(),
+				destination: "/tmp",
+			}
+			err := v.visit(tt.entry)
+			tt.wantErr(t, err)
+			if err != nil {
+				return
+			}
+			for _, fn := range tt.assertFs {
+				fn(t, v.fs)
+			}
+
+			// even if the test has no other assertions, check that the root is empty
+			assertNoFilesInRoot(t, v.fs)
+		})
+	}
+}

Vulnerable code: // UntarToDirectory writes the contents of the given tar reader to the given destination
	visitor := func(entry TarFileEntry) error {
		target := filepath.Join(dst, entry.Header.Name)

		switch entry.Header.Typeflag {
		case tar.TypeDir:
			if _, err := os.Stat(target); err != nil {
				if err := os.MkdirAll(target, 0755); err != nil {
					return err
				}
			}
		case tar.TypeReg:
			f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(entry.Header.Mode))
			if err != nil {
			// limit the reader on each file read to prevent decompression bomb attacks
			numBytes, err := io.Copy(f, io.LimitReader(entry.Reader, perFileReadLimit))
			if numBytes >= perFileReadLimit || errors.Is(err, io.EOF) {
				return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
			}
			if err != nil {
				return fmt.Errorf("unable to copy file: %w", err)
			}
			if err = f.Close(); err != nil {
				log.Errorf("failed to close file during untar of path=%q: %w", f.Name(), err)
			}
		return nil
	}
	return IterateTar(reader, visitor)
Fixed code: 	"strings"
	"github.com/spf13/afero"
// UntarToDirectory writes the contents of the given tar reader to the given destination. Note: this is meant to handle
// archives for images (not image contents) thus intentionally does not handle links or any kinds of special files.
	return IterateTar(
		reader,
		tarVisitor{
			fs:          afero.NewOsFs(),
			destination: dst,
		}.visit,
	)
}

type tarVisitor struct {
	fs          afero.Fs
	destination string
}

func (v tarVisitor) visit(entry TarFileEntry) error {
	target := filepath.Join(v.destination, entry.Header.Name)
	// we should not allow for any destination path to be outside of where we are unarchiving to
	if !strings.HasPrefix(target, v.destination) {
		return fmt.Errorf("potential path traversal attack with entry: %q", entry.Header.Name)
	}

	switch entry.Header.Typeflag {
	case tar.TypeSymlink, tar.TypeLink:
		// we don't handle this is to prevent any potential traversal attacks
		log.WithFields("path", entry.Header.Name).Trace("skipping symlink/link entry in image tar")

	case tar.TypeDir:
		if _, err := v.fs.Stat(target); err != nil {
			if err := v.fs.MkdirAll(target, 0755); err != nil {
		}
	case tar.TypeReg:
		f, err := v.fs.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(entry.Header.Mode))
		if err != nil {
			return err
		}
		// limit the reader on each file read to prevent decompression bomb attacks
		numBytes, err := io.Copy(f, io.LimitReader(entry.Reader, perFileReadLimit))
		if numBytes >= perFileReadLimit || errors.Is(err, io.EOF) {
			return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")
		}
		if err != nil {
			return fmt.Errorf("unable to copy file: %w", err)
		if err = f.Close(); err != nil {
			log.Errorf("failed to close file during untar of path=%q: %w", f.Name(), err)
		}
	}
	return nil
	"archive/tar"
	"sort"
	"strings"
	"github.com/google/go-cmp/cmp"
	"github.com/scylladb/go-set/strset"
	"github.com/spf13/afero"
	"github.com/stretchr/testify/require"

func Test_tarVisitor_visit(t *testing.T) {
	assertNoFilesInRoot := func(t testing.TB, fs afero.Fs) {
		t.Helper()

		allowableFiles := strset.New("tmp")

		// list all files in root
		files, err := afero.ReadDir(fs, "/")
		require.NoError(t, err)

		for _, f := range files {
			assert.True(t, allowableFiles.Has(f.Name()), "unexpected file in root: %s", f.Name())
		}
	}

	assertPaths := func(expectedFiles []string, expectedDirs []string) func(t testing.TB, fs afero.Fs) {
		return func(t testing.TB, fs afero.Fs) {
			t.Helper()

			sort.Strings(expectedFiles)
			haveFiles := strset.New()
			haveDirs := strset.New()
			err := afero.Walk(fs, "/", func(path string, info os.FileInfo, err error) error {
				require.NoError(t, err)
				if info.IsDir() {
					haveDirs.Add(path)
				} else {
					haveFiles.Add(path)
				}
				return nil
			})

			haveFilesList := haveFiles.List()
			sort.Strings(haveFilesList)

			haveDirsList := haveDirs.List()
			sort.Strings(haveDirsList)

			require.NoError(t, err)

			if d := cmp.Diff(expectedFiles, haveFilesList); d != "" {
				t.Errorf("unexpected files (-want +got):\n%s", d)
			}

			if d := cmp.Diff(expectedDirs, haveDirsList); d != "" {
				t.Errorf("unexpected dirs (-want +got):\n%s", d)
			}

		}
	}

	tests := []struct {
		name     string
		entry    TarFileEntry
		wantErr  require.ErrorAssertionFunc
		assertFs []func(t testing.TB, fs afero.Fs)
	}{
		{
			name: "regular file is written",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeReg,
					Name:     "file.txt",
					Linkname: "",
					Size:     2,
				},
				Reader: strings.NewReader("hi"),
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{"/tmp/file.txt"},
					[]string{"/", "/tmp"},
				),
			},
		},
		{
			name: "regular file with possible path traversal errors out",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeReg,
					Name:     "../file.txt",
					Linkname: "",
					Size:     2,
				},
				Reader: strings.NewReader("hi"),
			},
			wantErr: require.Error,
		},
		{
			name: "directory is created",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeDir,
					Name:     "dir",
					Linkname: "",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/", "/tmp", "/tmp/dir"},
				),
			},
		},
		{
			name: "symlink is ignored",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeSymlink,
					Name:     "symlink",
					Linkname: "./../to-location",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/"},
				),
			},
		},
		{
			name: "hardlink is ignored",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeLink,
					Name:     "link",
					Linkname: "./../to-location",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/"},
				),
			},
		},
		{
			name: "device is ignored",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeChar,
					Name:     "device",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/"},
				),
			},
		},
		{
			name: "block device is ignored",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeBlock,
					Name:     "device",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/"},
				),
			},
		},
		{
			name: "pipe is ignored",
			entry: TarFileEntry{
				Sequence: 0,
				Header: tar.Header{
					Typeflag: tar.TypeFifo,
					Name:     "pipe",
				},
			},
			assertFs: []func(t testing.TB, fs afero.Fs){
				assertPaths(
					[]string{},
					[]string{"/"},
				),
			},
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			if tt.wantErr == nil {
				tt.wantErr = require.NoError
			}
			v := tarVisitor{
				fs:          afero.NewMemMapFs(),
				destination: "/tmp",
			}
			err := v.visit(tt.entry)
			tt.wantErr(t, err)
			if err != nil {
				return
			}
			for _, fn := range tt.assertFs {
				fn(t, v.fs)
			}

			// even if the test has no other assertions, check that the root is empty
			assertNoFilesInRoot(t, v.fs)
		})
	}
}