feat: initial recursive archive cataloging impl

Signed-off-by: Keith Zantow <keith.zantow@anchore.com>

internal/file/copy.go

@@ -8,9 +8,9 @@ import (
 
 const perFileReadLimit = 2 * GB
 
-// safeCopy limits the copy from the reader. This is useful when extracting files from archives to
+// SafeCopy limits the copy from the reader. This is useful when extracting files from archives to
 // protect against decompression bomb attacks.
-func safeCopy(writer io.Writer, reader io.Reader) error {
+func SafeCopy(writer io.Writer, reader io.Reader) error {
 	numBytes, err := io.Copy(writer, io.LimitReader(reader, perFileReadLimit))
 	if numBytes >= perFileReadLimit {
 		return fmt.Errorf("zip read limit hit (potential decompression bomb attack)")

internal/file/copy_test.go

@@ -29,7 +29,7 @@ func (r *errReader) Read(p []byte) (int, error) {
 func TestSafeCopy(t *testing.T) {
 	t.Run("clean copy returns nil", func(t *testing.T) {
 		var buf bytes.Buffer
-		if err := safeCopy(&buf, strings.NewReader("hello")); err != nil {
+		if err := SafeCopy(&buf, strings.NewReader("hello")); err != nil {
 			t.Fatalf("unexpected error: %v", err)
 		}
 		if got := buf.String(); got != "hello" {
@@ -38,12 +38,12 @@ func TestSafeCopy(t *testing.T) {
 	})
 
 	t.Run("propagates decompression error", func(t *testing.T) {
-		// #4806: safeCopy used to drop non-EOF errors, so the caller
+		// #4806: SafeCopy used to drop non-EOF errors, so the caller
 		// would persist a partial buffer as a successful extract and
 		// downstream catalogers silently read empty manifests.
 		sentinel := errors.New("flate: corrupt input before offset 42")
 		var buf bytes.Buffer
-		err := safeCopy(&buf, &errReader{data: []byte("partial"), err: sentinel})
+		err := SafeCopy(&buf, &errReader{data: []byte("partial"), err: sentinel})
 		if err == nil {
 			t.Fatalf("expected error to be returned, got nil")
 		}
@@ -56,7 +56,7 @@ func TestSafeCopy(t *testing.T) {
 		// The old code had a dead io.EOF branch that labelled clean
 		// reads as decompression bombs; keep the happy path clean.
 		var buf bytes.Buffer
-		err := safeCopy(&buf, io.LimitReader(strings.NewReader("abc"), 3))
+		err := SafeCopy(&buf, io.LimitReader(strings.NewReader("abc"), 3))
 		if err != nil {
 			t.Fatalf("unexpected error on clean copy: %v", err)
 		}

internal/file/tar_file_traversal.go

@@ -70,7 +70,7 @@ func ExtractGlobsFromTarToUniqueTempFile(ctx context.Context, archivePath, dir s
 		}
 		defer internal.CloseAndLogError(packedFile, archivePath)
 
-		if err := safeCopy(tempFile, packedFile); err != nil {
+		if err := SafeCopy(tempFile, packedFile); err != nil {
 			return fmt.Errorf("unable to copy source=%q for tar=%q: %w", file.Name(), archivePath, err)
 		}
 

internal/file/zip_file_traversal.go

@@ -101,7 +101,7 @@ func ExtractFromZipToUniqueTempFile(ctx context.Context, archivePath, dir string
 			return fmt.Errorf("unable to extract directories, only files: %s", file.NameInArchive)
 		}
 
-		if err := safeCopy(tempFile, zippedFile); err != nil {
+		if err := SafeCopy(tempFile, zippedFile); err != nil {
 			return fmt.Errorf("unable to copy source=%q for zip=%q: %w", file.NameInArchive, archivePath, err)
 		}
 
@@ -138,7 +138,7 @@ func ContentsFromZip(ctx context.Context, archivePath string, paths ...string) (
 		}
 
 		var buffer bytes.Buffer
-		if err := safeCopy(&buffer, zippedFile); err != nil {
+		if err := SafeCopy(&buffer, zippedFile); err != nil {
 			return fmt.Errorf("unable to copy source=%q for zip=%q: %w", file.NameInArchive, archivePath, err)
 		}
 
@@ -165,10 +165,22 @@ func UnzipToDir(ctx context.Context, archivePath, targetDir string) error {
 }
 
 // SafeJoin ensures that any destinations do not resolve to a path above the prefix path.
+//
+// The comparison is on a path boundary rather than on the string prefix. A sibling directory that
+// happens to share the prefix as a string is not inside it: under "/tmp/x/contents" the destination
+// "../contents-evil/y" cleans to "/tmp/x/contents-evil/y", which starts with the prefix and is
+// outside it.
 func SafeJoin(prefix string, dest ...string) (string, error) {
 	joinResult := filepath.Join(append([]string{prefix}, dest...)...)
+	cleanPrefix := filepath.Clean(prefix)
 	cleanJoinResult := filepath.Clean(joinResult)
-	if !strings.HasPrefix(cleanJoinResult, filepath.Clean(prefix)) {
+
+	boundary := cleanPrefix
+	if !strings.HasSuffix(boundary, string(filepath.Separator)) {
+		boundary += string(filepath.Separator)
+	}
+
+	if cleanJoinResult != cleanPrefix && !strings.HasPrefix(cleanJoinResult, boundary) {
 		return "", &errZipSlipDetected{
 			Prefix:   prefix,
 			JoinArgs: dest,
@@ -210,7 +222,7 @@ func extractSingleFile(file archives.FileInfo, expandedFilePath, archivePath str
 			}
 		}()
 
-		if err := safeCopy(outputFile, zippedFile); err != nil {
+		if err := SafeCopy(outputFile, zippedFile); err != nil {
 			return fmt.Errorf("unable to copy source=%q to dest=%q for zip=%q: %w", file.NameInArchive, outputFile.Name(), archivePath, err)
 		}
 	}

internal/file/zip_file_traversal_test.go

@@ -305,6 +305,45 @@ func TestSafeJoin(t *testing.T) {
 			expected:     "",
 			errAssertion: assertErrorAs(&errZipSlipDetected{}),
 		},
+		// a legitimate nested path, several components deep
+		{
+			prefix: "/a/place",
+			args: []string{
+				"lib",
+				"sub",
+				"file.txt",
+			},
+			expected:     "/a/place/lib/sub/file.txt",
+			errAssertion: assert.NoError,
+		},
+		// the prefix itself is inside the prefix
+		{
+			prefix: "/a/place",
+			args: []string{
+				".",
+			},
+			expected:     "/a/place",
+			errAssertion: assert.NoError,
+		},
+		// siblings that share the prefix as a STRING are not inside it. A string-prefix comparison
+		// accepted these, so an archive entry could write into a directory next to the one it was
+		// being extracted into.
+		{
+			prefix: "/a/place",
+			args: []string{
+				"../place-evil/x",
+			},
+			expected:     "",
+			errAssertion: assertErrorAs(&errZipSlipDetected{}),
+		},
+		{
+			prefix: "/tmp/syft-archive-abc/contents",
+			args: []string{
+				"../contents-evil/x",
+			},
+			expected:     "",
+			errAssertion: assertErrorAs(&errZipSlipDetected{}),
+		},
 	}
 
 	for _, test := range tests {