From 842430d2d4e13b7c93abe94a82690f08b0888832 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Tue, 25 Aug 2026 10:14:27 +0100
Subject: [PATCH] archive: fix zip slip path traversal in untrusted zip files
 GHSA-66hp-wgxq-6f5q CVE-PENDING

The zip backend mounts a zip file as a browsable Fs. Go's archive/zip
does not sanitize entry names, and readZip applied path.Clean but did
not reject a cleaned name that still pointed outside the archive. A
crafted zip could make rclone copy/sync attempt writes outside the
intended destination.

Sanitize entry names with sanitize.Path - the same check used by
rclone archive extract - skipping any entry with a ".." path
component, whether separated by "/" or "\". A backslash is otherwise
kept as an ordinary character in the name, as archive extract does. It
is up to the destination backend to make names safe for its storage.

Skipped entries are logged as a single count per archive so a crafted
archive with many escaping entries cannot flood the log.
---
 backend/archive/zip/zip.go               | 13 +++-
 backend/archive/zip/zip_internal_test.go | 85 ++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 3 deletions(-)
 create mode 100644 backend/archive/zip/zip_internal_test.go

diff --git a/backend/archive/zip/zip.go b/backend/archive/zip/zip.go
index 17511b76b..ec9ba77d7 100644
--- a/backend/archive/zip/zip.go
+++ b/backend/archive/zip/zip.go
@@ -18,6 +18,7 @@ import (
 	"github.com/rclone/rclone/fs/hash"
 	"github.com/rclone/rclone/fs/log"
 	"github.com/rclone/rclone/lib/readers"
+	"github.com/rclone/rclone/lib/sanitize"
 	"github.com/rclone/rclone/vfs"
 	"github.com/rclone/rclone/vfs/vfscommon"
 )
@@ -135,10 +136,13 @@ func (f *Fs) readZip() (singleObject bool, err error) {
 		return singleObject, fmt.Errorf("failed to read zip file: %w", err)
 	}
 	dt := dirtree.New()
+	skipped := 0
 	for _, file := range zr.File {
-		remote := strings.Trim(path.Clean(file.Name), "/")
-		if remote == "." {
-			remote = ""
+		// Skip entries whose name escapes the archive's own namespace
+		remote, err := sanitize.Path(file.Name)
+		if err != nil {
+			skipped++
+			continue
 		}
 		remote = path.Join(f.prefix, remote)
 		if f.root != "" {
@@ -173,6 +177,9 @@ func (f *Fs) readZip() (singleObject bool, err error) {
 			}
 		}
 	}
+	if skipped > 0 {
+		fs.Logf(f, "Skipped %d zip entries which escape the archive", skipped)
+	}
 	dt.CheckParents("")
 	dt.Sort()
 	f.dt = dt
diff --git a/backend/archive/zip/zip_internal_test.go b/backend/archive/zip/zip_internal_test.go
new file mode 100644
index 000000000..f0bb3bb03
--- /dev/null
+++ b/backend/archive/zip/zip_internal_test.go
@@ -0,0 +1,85 @@
+package zip
+
+import (
+	"archive/zip"
+	"bytes"
+	"context"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"testing"
+
+	_ "github.com/rclone/rclone/backend/local"
+	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/cache"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// writeZip builds a zip file with the given entry names (a name ending
+// in "/" is a directory) in dir and returns its leaf name.
+func writeZip(t *testing.T, dir, name string, names ...string) string {
+	var buf bytes.Buffer
+	zw := zip.NewWriter(&buf)
+	for _, n := range names {
+		w, err := zw.Create(n)
+		require.NoError(t, err)
+		if !strings.HasSuffix(n, "/") {
+			_, err = w.Write([]byte("data for " + n))
+			require.NoError(t, err)
+		}
+	}
+	require.NoError(t, zw.Close())
+	require.NoError(t, os.WriteFile(filepath.Join(dir, name), buf.Bytes(), 0600))
+	return name
+}
+
+// allRemotes returns every Object remote in the mounted archive's dirtree.
+func allRemotes(t *testing.T, f fs.Fs) []string {
+	zf, ok := f.(*Fs)
+	require.True(t, ok)
+	var remotes []string
+	for _, entries := range zf.dt {
+		for _, entry := range entries {
+			if o, ok := entry.(*Object); ok {
+				remotes = append(remotes, o.Remote())
+			}
+		}
+	}
+	// dt iteration order is nondeterministic, so sort for stable comparison.
+	sort.Strings(remotes)
+	return remotes
+}
+
+// A malicious zip whose entry names escape the archive's own namespace
+// must not be exposed by the zip backend (Zip Slip).
+func TestReadZipSlip(t *testing.T) {
+	ctx := context.Background()
+	dir := t.TempDir()
+	name := writeZip(t, dir, "evil.zip",
+		"good.txt",
+		"../../etc/cron.d/evil",
+		"../escape.txt",
+		"sub/../../above.txt",
+		`..\..\evil.exe`,
+	)
+
+	localFs, err := cache.Get(ctx, dir)
+	require.NoError(t, err)
+
+	f, err := New(ctx, localFs, name, "", "")
+	require.NoError(t, err)
+
+	remotes := allRemotes(t, f)
+
+	// The one benign entry must survive.
+	assert.Contains(t, remotes, "good.txt")
+
+	// No escaping entry may be exposed.
+	for _, remote := range remotes {
+		assert.False(t, remote == ".." || strings.HasPrefix(remote, "../"),
+			"escaping remote exposed: %q", remote)
+	}
+	assert.Equal(t, []string{"good.txt"}, remotes)
+}