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