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

@@ -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

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