fix: prevent path traversal in tar extraction

Signed-off-by: Chuanhao Jin <15221580643@163.com>

go.mod

@@ -116,7 +116,7 @@ require (
 	github.com/containerd/ttrpc v1.2.5 // indirect
 	github.com/coreos/go-semver v0.3.1 // indirect
 	github.com/coreos/go-systemd/v22 v22.5.0 // indirect
-	github.com/cyphar/filepath-securejoin v0.3.6 // indirect
+	github.com/cyphar/filepath-securejoin v0.3.6
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/distribution/reference v0.6.0
 	github.com/docker/cli v25.0.1+incompatible // indirect

keadm/cmd/keadm/app/cmd/util/common.go

@@ -24,11 +24,14 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
 
+	securejoin "github.com/cyphar/filepath-securejoin"
+
 	"github.com/blang/semver"
 	"github.com/spf13/pflag"
 	versionutil "k8s.io/apimachinery/pkg/util/version"
@@ -228,7 +231,12 @@ func DecompressTarGz(gzFilePath, dest string) error {
 	}
 	defer reader.Close()
 
-	err = os.MkdirAll(dest, os.ModePerm)
+	absDest, err := filepath.Abs(dest)
+	if err != nil {
+		return err
+	}
+
+	err = os.MkdirAll(absDest, os.ModePerm)
 	if err != nil {
 		return err
 	}
@@ -252,7 +260,23 @@ func DecompressTarGz(gzFilePath, dest string) error {
 			continue
 		}
 
-		target := filepath.Join(dest, header.Name)
+		if header.Name == "" {
+			return fmt.Errorf("tar entry name is empty")
+		}
+
+		entryName := strings.ReplaceAll(header.Name, "\\", "/")
+		entryName = path.Clean(entryName)
+		if len(entryName) >= 2 && entryName[1] == ':' && ((entryName[0] >= 'a' && entryName[0] <= 'z') || (entryName[0] >= 'A' && entryName[0] <= 'Z')) {
+			return fmt.Errorf("tar entry %q attempts path traversal outside %s", header.Name, absDest)
+		}
+		if path.IsAbs(entryName) || entryName == ".." || strings.HasPrefix(entryName, "../") {
+			return fmt.Errorf("tar entry %q attempts path traversal outside %s", header.Name, absDest)
+		}
+
+		target, err := securejoin.SecureJoin(absDest, entryName)
+		if err != nil {
+			return err
+		}
 		switch header.Typeflag {
 		case tar.TypeDir:
 			if _, err := os.Stat(target); err != nil {

keadm/cmd/keadm/app/cmd/util/common_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package util
 
 import (
+	"archive/tar"
+	"compress/gzip"
 	"errors"
 	"fmt"
 	"os"
@@ -932,3 +934,126 @@ func TestCleanupCompressFile(t *testing.T) {
 		t.Logf("Expected error: %v", err)
 	}
 }
+
+type testTarEntry struct {
+	name     string
+	body     string
+	typeflag byte
+}
+
+func createTestTarGz(t *testing.T, entries []testTarEntry) string {
+	t.Helper()
+
+	archivePath := filepath.Join(t.TempDir(), "test.tar.gz")
+	file, err := os.Create(archivePath)
+	if err != nil {
+		t.Fatalf("failed to create test archive: %v", err)
+	}
+
+	gzWriter := gzip.NewWriter(file)
+	tarWriter := tar.NewWriter(gzWriter)
+
+	for _, entry := range entries {
+		header := &tar.Header{
+			Name:     entry.name,
+			Mode:     0644,
+			Typeflag: entry.typeflag,
+		}
+		if entry.typeflag == tar.TypeReg {
+			header.Size = int64(len(entry.body))
+		}
+		if entry.typeflag == tar.TypeDir {
+			header.Mode = 0755
+		}
+
+		if err := tarWriter.WriteHeader(header); err != nil {
+			t.Fatalf("failed to write tar header: %v", err)
+		}
+		if entry.typeflag == tar.TypeReg {
+			if _, err := tarWriter.Write([]byte(entry.body)); err != nil {
+				t.Fatalf("failed to write tar body: %v", err)
+			}
+		}
+	}
+
+	if err := tarWriter.Close(); err != nil {
+		t.Fatalf("failed to close tar writer: %v", err)
+	}
+	if err := gzWriter.Close(); err != nil {
+		t.Fatalf("failed to close gzip writer: %v", err)
+	}
+	if err := file.Close(); err != nil {
+		t.Fatalf("failed to close archive file: %v", err)
+	}
+
+	return archivePath
+}
+
+func TestDecompressTarGzAllowsSafeEntries(t *testing.T) {
+	dest := filepath.Join(t.TempDir(), "extract")
+	archivePath := createTestTarGz(t, []testTarEntry{
+		{name: "dir", typeflag: tar.TypeDir},
+		{name: "dir/file.txt", body: "SAFE", typeflag: tar.TypeReg},
+	})
+
+	err := DecompressTarGz(archivePath, dest)
+	assert.NoError(t, err)
+
+	got, err := os.ReadFile(filepath.Join(dest, "dir", "file.txt"))
+	assert.NoError(t, err)
+	assert.Equal(t, "SAFE", string(got))
+}
+
+func TestDecompressTarGzRejectsPathTraversal(t *testing.T) {
+	tests := []struct {
+		name      string
+		entryName string
+	}{
+		{
+			name:      "parent directory traversal",
+			entryName: "../victim.txt",
+		},
+		{
+			name:      "backslash parent directory traversal",
+			entryName: `..\victim.txt`,
+		},
+		{
+			name:      "windows drive absolute path",
+			entryName: `C:\Users\Public\victim.txt`,
+		},
+		{
+			name:      "windows drive absolute path with slashes",
+			entryName: "C:/Users/Public/victim.txt",
+		},
+		{
+			name:      "absolute unix path",
+			entryName: "/tmp/victim.txt",
+		},
+		{
+			name:      "nested parent traversal",
+			entryName: "dir/../../victim.txt",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			base := t.TempDir()
+			dest := filepath.Join(base, "extract")
+			victim := filepath.Join(base, "victim.txt")
+
+			err := os.WriteFile(victim, []byte("ORIGINAL"), 0644)
+			assert.NoError(t, err)
+
+			archivePath := createTestTarGz(t, []testTarEntry{
+				{name: tt.entryName, body: "OVERWRITTEN", typeflag: tar.TypeReg},
+			})
+
+			err = DecompressTarGz(archivePath, dest)
+			assert.Error(t, err)
+
+			got, err := os.ReadFile(victim)
+			assert.NoError(t, err)
+			assert.Equal(t, "ORIGINAL", string(got))
+		})
+	}
+}