serve docker: reject volume names that escape the base directory GHSA-p6vx-hf7p-98j6
A Docker VolumeDriver.Create request carries a raw volume name that was joined onto the base directory with filepath.Join and used verbatim as the mountpoint. filepath.Join collapses ".." components, so a crafted name such as "../../../etc/foo" resolved to a host path outside the base directory, where the plugin then created a directory and mounted the remote. Confine the mountpoint to the base directory and refuse any name that resolves outside it.
This commit is contained in:
@@ -204,6 +204,49 @@ func TestDockerPluginLogic(t *testing.T) {
|
||||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
// TestDockerPluginVolumeNameConfined checks that a crafted volume name
|
||||
// containing ".." components cannot escape the base directory to create a
|
||||
// mountpoint at an arbitrary host path.
|
||||
func TestDockerPluginVolumeNameConfined(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)
|
||||
}
|
||||
}()
|
||||
|
||||
drv, err := docker.NewDriver(ctx, testDir, nil, nil, true, true)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, drv)
|
||||
|
||||
// A volume name with ".." components resolves outside testDir, to a
|
||||
// sibling path that does not exist so the test never touches a real
|
||||
// system directory.
|
||||
escape := filepath.Join("..", "docker-escape-target")
|
||||
outside := filepath.Join(filepath.Dir(testDir), "docker-escape-target")
|
||||
require.NoDirExists(t, outside)
|
||||
volReq := &docker.CreateRequest{
|
||||
Name: escape,
|
||||
Options: docker.VolOpts{"remote": testDir},
|
||||
}
|
||||
err = drv.Create(volReq)
|
||||
assertErrorContains(t, err, "resolves outside the base directory")
|
||||
|
||||
// The escaping directory must not have been created.
|
||||
_, statErr := os.Stat(outside)
|
||||
assert.True(t, os.IsNotExist(statErr), "escaping mountpoint %q should not exist", outside)
|
||||
|
||||
// The volume must not have been registered.
|
||||
_, err = drv.Get(&docker.GetRequest{Name: escape})
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
const (
|
||||
httpTimeout = 2 * time.Second
|
||||
tempDelay = 10 * time.Millisecond
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"sort"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/rclone/rclone/cmd/mountlib"
|
||||
@@ -54,8 +55,24 @@ type VolInfo struct {
|
||||
Status map[string]any `json:",omitempty"`
|
||||
}
|
||||
|
||||
// volumeMountPath returns the mountpoint for a volume called name below
|
||||
// root, together with whether that path stays confined within root.
|
||||
//
|
||||
// filepath.Join cleans its result, collapsing any ".." components in the
|
||||
// volume name, so a crafted name could otherwise resolve to an arbitrary
|
||||
// host path outside root. Callers must reject names for which confined is
|
||||
// false rather than creating a directory and mounting there.
|
||||
func volumeMountPath(root, name string) (path string, confined bool) {
|
||||
path = filepath.Join(root, name)
|
||||
root = filepath.Clean(root)
|
||||
return path, path == root || strings.HasPrefix(path, root+string(filepath.Separator))
|
||||
}
|
||||
|
||||
func newVolume(ctx context.Context, name string, volOpt VolOpts, drv *Driver) (*Volume, error) {
|
||||
path := filepath.Join(drv.root, name)
|
||||
path, confined := volumeMountPath(drv.root, name)
|
||||
if !confined {
|
||||
return nil, fmt.Errorf("invalid volume name %q: resolves outside the base directory", name)
|
||||
}
|
||||
mnt := &mountlib.MountPoint{
|
||||
MountPoint: path,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user