From f5795d00c27ccda7dbdc1b87e697c94f911482d9 Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Thu, 20 Aug 2026 11:59:21 +0100
Subject: [PATCH] serve docker: re-derive volume mountpoint from name when
 restoring state GHSA-p6vx-hf7p-98j6

When the plugin restarts it reads its persisted state file and used the
stored mountpoint verbatim. A state file written by an older rclone that
allowed escaping volume names, or one that was tampered with, could point
the mountpoint outside the base directory, so upgrading did not remediate
an already-escaped volume.

Re-derive the mountpoint from the base directory and the volume name on
restore, confined to the base directory, rather than trusting the stored
path.
---
 cmd/serve/docker/docker_test.go | 42 +++++++++++++++++++++++++++++++++
 cmd/serve/docker/volume.go      | 11 ++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/cmd/serve/docker/docker_test.go b/cmd/serve/docker/docker_test.go
index c695ae21e..7cfa77560 100644
--- a/cmd/serve/docker/docker_test.go
+++ b/cmd/serve/docker/docker_test.go
@@ -247,6 +247,48 @@ func TestDockerPluginVolumeNameConfined(t *testing.T) {
 	assert.Error(t, err)
 }
 
+// TestDockerPluginRestoreStateConfined checks that a persisted state file
+// with a mountpoint escaping the base directory is not trusted verbatim on
+// restore: the mountpoint is re-derived from the base directory and name.
+func TestDockerPluginRestoreStateConfined(t *testing.T) {
+	ctx := context.Background()
+	oldCacheDir := config.GetCacheDir()
+	testDir, testFs := initialise(ctx, t)
+	err := config.SetCacheDir(testDir)
+	require.NoError(t, err)
+	defer func() {
+		_ = config.SetCacheDir(oldCacheDir)
+		if !t.Failed() {
+			fstest.Purge(testFs)
+			_ = os.RemoveAll(testDir)
+		}
+	}()
+
+	// Persist a state file as an older, vulnerable rclone might have: a
+	// benign name but a mountpoint that escapes the base directory.
+	escaped := filepath.Join(filepath.Dir(testDir), "docker-escape-restore")
+	require.NoDirExists(t, escaped)
+	state := fmt.Sprintf(`[{"name":"vol1","mountpoint":%q,"created":%q,"fs":%q,"options":{"remote":%q},"mounts":[]}]`,
+		escaped, time.Now().Format(time.RFC3339), testDir, testDir)
+	statePath := filepath.Join(testDir, "docker-plugin.state")
+	require.NoError(t, os.WriteFile(statePath, []byte(state), 0600))
+
+	// Restore the state into a new dummy driver.
+	drv, err := docker.NewDriver(ctx, testDir, nil, nil, true, false)
+	require.NoError(t, err)
+	require.NotNil(t, drv)
+
+	// The restored volume must point back inside the base directory, and
+	// the escaping directory must not have been created.
+	getRes, err := drv.Get(&docker.GetRequest{Name: "vol1"})
+	require.NoError(t, err)
+	require.NotNil(t, getRes)
+	assert.Equal(t, filepath.Join(testDir, "vol1"), getRes.Volume.Mountpoint)
+	assert.NotEqual(t, escaped, getRes.Volume.Mountpoint)
+	_, statErr := os.Stat(escaped)
+	assert.True(t, os.IsNotExist(statErr), "escaping mountpoint %q should not exist", escaped)
+}
+
 const (
 	httpTimeout = 2 * time.Second
 	tempDelay   = 10 * time.Millisecond
diff --git a/cmd/serve/docker/volume.go b/cmd/serve/docker/volume.go
index 813ab6320..e5b458a34 100644
--- a/cmd/serve/docker/volume.go
+++ b/cmd/serve/docker/volume.go
@@ -121,8 +121,17 @@ func (vol *Volume) prepareState() {
 // retrieved with getPendingMounts for deferred mounting.
 func (vol *Volume) restoreState(ctx context.Context, drv *Driver) error {
 	vol.drv = drv
+	// Re-derive the mountpoint from the base directory and name rather
+	// than trusting the persisted path, which an older rclone or a
+	// tampered state file could have left pointing outside the base
+	// directory.
+	path, confined := volumeMountPath(drv.root, vol.Name)
+	if !confined {
+		return fmt.Errorf("invalid volume name %q: resolves outside the base directory", vol.Name)
+	}
+	vol.MountPoint = path
 	vol.mnt = &mountlib.MountPoint{
-		MountPoint: vol.MountPoint,
+		MountPoint: path,
 	}
 	// Save pending mounts before applyOptions clears them
 	vol.pendingMounts = vol.Mounts