archive extract: fix path traversal letting archives escape the destination CVE-2026-59732

Archive entry names are attacker controlled. `rclone archive extract` stripped
only a leading `./` and then joined the entry name onto the destination
directory with `path.Join`, which collapses `..` segments. An entry such as
`../escaped.txt` extracted into `:s3:bucket/safe/prefix` therefore resolved to
`bucket/safe/escaped.txt`, outside the selected `prefix` directory - a path
traversal ("Zip Slip") attack that could create or overwrite sibling objects on
any destination remote.

Entry names are now validated before use: a leading `./` is still stripped (tar
archives created with `tar -czf archive.tar.gz .` rely on this), but any entry
with a `..` path component is rejected. Both `/` and `\` are treated as
separators when looking for `..`, as the local backend treats `\` as a path
separator on Windows.

Fixes: GHSA-4vr5-p2gc-h23p

cmd/archive/archive_test.go

@@ -1,6 +1,8 @@
 package archive_test
 
 import (
+	"archive/zip"
+	"bytes"
 	"context"
 	"strings"
 	"testing"
@@ -170,6 +172,60 @@ func TestIntegration(t *testing.T) {
 	testArchive(t)
 }
 
+// craftZip builds an in-memory zip archive with the given entries, returning
+// it as a string. The entry names are written verbatim so they can include
+// crafted path traversal sequences that the archive create command would
+// never produce from real files.
+func craftZip(t *testing.T, entries []struct{ name, content string }) string {
+	var buf bytes.Buffer
+	zw := zip.NewWriter(&buf)
+	for _, e := range entries {
+		w, err := zw.CreateHeader(&zip.FileHeader{Name: e.name, Method: zip.Store})
+		require.NoError(t, err)
+		_, err = w.Write([]byte(e.content))
+		require.NoError(t, err)
+	}
+	require.NoError(t, zw.Close())
+	return buf.String()
+}
+
+// TestExtractPathTraversal checks that a crafted archive whose entries try to
+// escape the destination directory with ".." components is rejected and does
+// not write anything outside the destination (GHSA-4vr5-p2gc-h23p).
+func TestExtractPathTraversal(t *testing.T) {
+	ctx := context.Background()
+
+	for _, tc := range []struct {
+		name  string
+		entry string
+	}{
+		{name: "forward slash", entry: "../escaped.txt"},
+		{name: "backslash", entry: `..\escaped.txt`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			r := fstest.NewRun(t)
+
+			zipData := craftZip(t, []struct{ name, content string }{
+				{"safe.txt", "safe content"},
+				{tc.entry, "escaped content"},
+			})
+			r.WriteObject(ctx, "malicious.zip", zipData, t1)
+
+			// Extract into a subdirectory so a successful escape would land
+			// in "safe/escaped.txt", a sibling of the "safe/prefix" target.
+			err := extract.ArchiveExtract(ctx, r.Flocal, "safe/prefix", r.Fremote, "malicious.zip")
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "..")
+
+			// Nothing should have escaped the destination directory.
+			for _, escaped := range []string{"safe/escaped.txt", "escaped.txt"} {
+				_, err := r.Flocal.NewObject(ctx, escaped)
+				assert.ErrorIs(t, err, fs.ErrorObjectNotFound, escaped)
+			}
+		})
+	}
+}
+
 func TestMemory(t *testing.T) {
 	if *fstest.RemoteName != "" {
 		t.Skip("skipping as -remote is set")

cmd/archive/extract/extract.go

@@ -150,19 +150,13 @@ func ArchiveExtract(ctx context.Context, dst fs.Fs, dstDir string, src fs.Fs, sr
 	}
 	// extract files
 	err = ex.Extract(ctx, in, func(ctx context.Context, f archives.FileInfo) error {
-		remote := f.NameInArchive
-		// Strip leading "./" from archive paths. Tar files created with
-		// relative paths (e.g. "tar -czf archive.tar.gz .") use "./" prefixed
-		// entries. Without stripping, rclone encodes the "." as a full-width
-		// dot character creating a spurious directory. We only strip "./"
-		// specifically to avoid enabling path traversal attacks via "../".
-		remote = strings.TrimPrefix(remote, "./")
-		// If the entry was exactly "./" (the root dir), skip it
-		if remote == "" && f.IsDir() {
-			return nil
+		remote, err := destPath(f.NameInArchive, dstDir)
+		if err != nil {
+			return err
 		}
-		if dstDir != "" {
-			remote = path.Join(dstDir, remote)
+		// Skip the archive root entry ("./") which has no name of its own.
+		if remote == "" {
+			return nil
 		}
 		// check if file should be extracted
 		if !fi.Include(remote, f.Size(), f.ModTime(), fs.Metadata{}) {
@@ -203,3 +197,38 @@ func ArchiveExtract(ctx context.Context, dst fs.Fs, dstDir string, src fs.Fs, sr
 
 	return err
 }
+
+// destPath maps an archive entry name onto its destination remote within
+// dstDir, returning an error if the name is unsafe.
+//
+// Archive entry names are attacker controlled. A leading "./" is stripped:
+// tar archives created with relative paths (e.g. "tar -czf archive.tar.gz .")
+// use "./" prefixed entries and, without stripping, rclone would encode the
+// "." as a full-width dot character creating a spurious directory.
+//
+// Any entry with a ".." path component is then rejected to prevent a path
+// traversal ("Zip Slip") attack: path.Join collapses "..", so an entry such
+// as "../escaped.txt" joined onto "dir" would resolve to "escaped.txt" and be
+// written outside the selected destination directory. Both "/" and "\" are
+// treated as separators when looking for ".." segments: archive names should
+// use "/", but a crafted archive may use "\", which the local backend treats
+// as a path separator on Windows.
+//
+// The returned remote is empty for the archive root entry ("./"), which the
+// caller should skip.
+func destPath(nameInArchive, dstDir string) (string, error) {
+	remote := strings.TrimPrefix(nameInArchive, "./")
+	isSeparator := func(r rune) bool { return r == '/' || r == '\\' }
+	for _, segment := range strings.FieldsFunc(remote, isSeparator) {
+		if segment == ".." {
+			return "", fmt.Errorf("refusing to extract archive entry %q with a %q path component", nameInArchive, "..")
+		}
+	}
+	if remote == "" {
+		return "", nil
+	}
+	if dstDir != "" {
+		remote = path.Join(dstDir, remote)
+	}
+	return remote, nil
+}

cmd/archive/extract/extract_test.go

@@ -3,17 +3,19 @@
 package extract
 
 import (
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestStripDotSlashPrefix(t *testing.T) {
+func TestDestPath(t *testing.T) {
 	tests := []struct {
 		name     string
 		input    string
+		dstDir   string
 		expected string
+		wantErr  bool
 	}{
 		{
 			name:     "strip leading dot-slash from file",
@@ -36,12 +38,13 @@ func TestStripDotSlashPrefix(t *testing.T) {
 			expected: "dir/file.txt",
 		},
 		{
-			name:     "dot-dot-slash NOT stripped (path traversal safety)",
-			input:    "../etc/passwd",
-			expected: "../etc/passwd",
+			name:     "joined onto destination directory",
+			input:    "file.txt",
+			dstDir:   "safe/prefix",
+			expected: "safe/prefix/file.txt",
 		},
 		{
-			name:     "dot-slash directory entry becomes empty",
+			name:     "archive root entry skipped",
 			input:    "./",
 			expected: "",
 		},
@@ -50,12 +53,49 @@ func TestStripDotSlashPrefix(t *testing.T) {
 			input:    "././file.txt",
 			expected: "./file.txt",
 		},
+		{
+			name:    "leading dot-dot rejected",
+			input:   "../etc/passwd",
+			wantErr: true,
+		},
+		{
+			name:    "leading dot-dot rejected with destination",
+			input:   "../escaped.txt",
+			dstDir:  "safe/prefix",
+			wantErr: true,
+		},
+		{
+			name:    "interior dot-dot rejected",
+			input:   "dir/../../escaped.txt",
+			dstDir:  "safe/prefix",
+			wantErr: true,
+		},
+		{
+			name:    "trailing dot-dot rejected",
+			input:   "dir/..",
+			wantErr: true,
+		},
+		{
+			name:    "backslash dot-dot rejected",
+			input:   `..\escaped.txt`,
+			wantErr: true,
+		},
+		{
+			name:    "nested backslash dot-dot rejected",
+			input:   `dir\..\..\escaped.txt`,
+			dstDir:  "safe/prefix",
+			wantErr: true,
+		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			// This mirrors the stripping logic in ArchiveExtract
-			got := strings.TrimPrefix(tc.input, "./")
+			got, err := destPath(tc.input, tc.dstDir)
+			if tc.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
 			assert.Equal(t, tc.expected, got)
 		})
 	}