fix: bug in prefix and suffix check using filepath

internal/unpackinfo/unpackinfo.go

@@ -30,20 +30,21 @@ type UnpackInfo struct {
 // It will return an error if the header represents an illegal symlink extraction
 // or if the entry type is not supported by go-slug.
 func NewUnpackInfo(dst string, header *tar.Header) (UnpackInfo, error) {
-	// Get rid of absolute paths.
-	path := header.Name
-
-	if path[0] == '/' {
-		path = strings.TrimPrefix(path, "/")
+	// Check for empty destination
+	if len(dst) == 0 {
+		return UnpackInfo{}, errors.New("empty destination is not allowed")
 	}
-	path = filepath.Join(dst, path)
 
-	// Check for paths outside our directory, they are forbidden
-	if len(dst) > 0 && !strings.HasSuffix(dst, "/") {
-		dst += "/"
-	}
+	// Clean the destination path
+	dst = filepath.Clean(dst)
+	path := filepath.Clean(header.Name)
+
+	path = filepath.Join(dst, path)
 	target := filepath.Clean(path)
-	if !strings.HasPrefix(target, dst) {
+
+	// Check for path traversal by ensuring the target is within the destination
+	rel, err := filepath.Rel(dst, target)
+	if err != nil || strings.HasPrefix(rel, "..") {
 		return UnpackInfo{}, errors.New("invalid filename, traversal with \"..\" outside of current directory")
 	}
 

internal/unpackinfo/unpackinfo_test.go

@@ -7,6 +7,7 @@ import (
 	"archive/tar"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -111,6 +112,168 @@ func TestNewUnpackInfo(t *testing.T) {
 			t.Fatalf("expected error to contain %q, got %q", expected, err)
 		}
 	})
+	t.Run("path starting with ./", func(t *testing.T) {
+		dst := t.TempDir()
+		result, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "./test/foo.txt",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+
+		expected := dst + "/test/foo.txt"
+		if result.Path != expected {
+			t.Fatalf("expected error to contain %q, got %q", expected, result.Path)
+		}
+	})
+	t.Run("path starting with ./ followed with ../", func(t *testing.T) {
+		dst := t.TempDir()
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "./../../test/foo.txt",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err == nil {
+			t.Fatal("expected error, got nil")
+		}
+
+		expected := "traversal with \"..\" outside of current"
+		if !strings.Contains(err.Error(), expected) {
+			t.Fatalf("expected error to contain %q, got %q", expected, err)
+		}
+	})
+	t.Run("destination starting with ./", func(t *testing.T) {
+		dst := t.TempDir()
+		outsideDst := "./" + dst
+		result, err := NewUnpackInfo(outsideDst, &tar.Header{
+			Name:     "foo.txt",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+
+		expected := filepath.Join(outsideDst, "foo.txt")
+		if expected != result.Path {
+			t.Fatalf("expected error to contain %q, got %q", expected, result.Path)
+		}
+	})
+	t.Run("empty destination", func(t *testing.T) {
+		emptyDestination := ""
+		_, err := NewUnpackInfo(emptyDestination, &tar.Header{
+			Name:     "foo.txt",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err == nil {
+			t.Fatal("expected error, got nil")
+		}
+
+		expected := "empty destination is not allowed"
+		if !strings.Contains(err.Error(), expected) {
+			t.Fatalf("expected error to contain %q, got %q", expected, err)
+		}
+	})
+	t.Run("valid empty path", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid empty path with destination without the / sufix", func(t *testing.T) {
+		dst := t.TempDir()
+		dst = strings.TrimSuffix(dst, "/")
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid path multiple / prefix", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "///////foo",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid path with / sufix", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "foo/",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid destination with / prefix", func(t *testing.T) {
+		dst := "/" + t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "foo/",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid symlink", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "foo.txt",
+			Typeflag: tar.TypeSymlink,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid file", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "foo.txt",
+			Typeflag: tar.TypeReg,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
+	t.Run("valid directory", func(t *testing.T) {
+		dst := t.TempDir()
+
+		_, err := NewUnpackInfo(dst, &tar.Header{
+			Name:     "foo",
+			Typeflag: tar.TypeDir,
+		})
+
+		if err != nil {
+			t.Fatalf("expected nil, got %q", err)
+		}
+	})
 }
 
 func TestUnpackInfo_RestoreInfo(t *testing.T) {