diff --git a/main.go b/main.go index e7a31f3..823a691 100644 --- a/main.go +++ b/main.go @@ -369,8 +369,8 @@ func allowedRoots(home string) (roots []string, def bool, err error) { return nil, false, fmt.Errorf("%s: %q is a dot-directory of $HOME", allowedPathsFile(), l) } for _, bad := range forbiddenRoots { - if inside(r, bad) { - return nil, false, fmt.Errorf("%s: %q is under %s", allowedPathsFile(), l, bad) + if bad = realpath(bad); inside(r, bad) || inside(bad, r) { + return nil, false, fmt.Errorf("%s: %q overlaps %s", allowedPathsFile(), l, bad) } } for _, d := range pathDirs() { @@ -398,9 +398,9 @@ func pathDirs() []string { } // checkLaunchDir returns pwd resolved if it is strictly under an allowed root, -// never $HOME itself or anything holding the sandbox's config, the API socket -// or a $PATH entry. Under the default $HOME root, dot-dirs of $HOME are -// refused too. +// never $HOME itself, anything holding the sandbox's config, the API socket or +// a $PATH entry, nor anything overlapping a forbidden system dir. Under the +// default $HOME root, dot-dirs of $HOME are refused too. func checkLaunchDir(pwd, allowlist string) (string, error) { pwd = realpath(pwd) home := realpath(os.Getenv("HOME")) @@ -416,11 +416,16 @@ func checkLaunchDir(pwd, allowlist string) (string, error) { } paths := allowedPathsFile() for _, dir := range append([]string{configDir(), filepath.Dir(allowlist), filepath.Dir(realpath(allowlist)), - filepath.Dir(paths), filepath.Dir(realpath(paths)), runtimeDir()}, pathDirs()...) { + filepath.Dir(realpath(paths)), runtimeDir()}, pathDirs()...) { if dir = realpath(dir); inside(dir, pwd) { return "", fmt.Errorf("refusing to mount %s: it contains %s", pwd, dir) } } + for _, bad := range forbiddenRoots { + if bad = realpath(bad); inside(pwd, bad) || inside(bad, pwd) { + return "", fmt.Errorf("refusing to mount %s: it overlaps %s", pwd, bad) + } + } for _, r := range roots { if pwd != r && inside(pwd, r) { return pwd, nil diff --git a/main_test.go b/main_test.go index db257ca..753e279 100644 --- a/main_test.go +++ b/main_test.go @@ -325,6 +325,7 @@ func TestLaunchRefusals(t *testing.T) { writePaths(t, e, "~/src") }}, {"invalid-root", "home/proj", func(t *testing.T, e *env) { writePaths(t, e, "/run/x") }}, + {"var-tmp-under-var-root", "/var/tmp", func(t *testing.T, e *env) { writePaths(t, e, "/var") }}, {"default-root-dot-dir", "home/.local/src/x", func(t *testing.T, e *env) { _ = os.MkdirAll(e.work+"/home/.local/src/x", 0o755) }}, @@ -359,6 +360,11 @@ func TestCheckLaunchDirRuntimeFallback(t *testing.T) { } t.Setenv("HOME", "/tmp/home") _, err = checkLaunchDir("/tmp/home/proj", "/nonexistent/hosts") + check(t, "forbidden default root", true, err != nil && strings.Contains(err.Error(), "overlaps "+realpath("/tmp"))) + origRoots := forbiddenRoots + forbiddenRoots = []string{"/run"} + t.Cleanup(func() { forbiddenRoots = origRoots }) + _, err = checkLaunchDir("/tmp/home/proj", "/nonexistent/hosts") check(t, "subdir", nil, err) } @@ -591,11 +597,14 @@ func TestCheckLaunchDirRoots(t *testing.T) { } for _, bad := range []string{"/", "/home", "~", "~/", "~/.local", "~/.local/share", "~/.ssh", "~/.ssh/x", "~/.gnupg", "~/.config/foo", cfg, cfg + "/agent-sandbox/x", "~/.local/share/containers/s", "relative/x", "/run/user/1000", "/tmp/x", "/var/tmp", "/proc/1", - "/sys", "/dev/shm", "/usr", "/srv/tools"} { + "/sys", "/dev/shm", "/usr", "/srv/tools", "/var"} { write("/srv/work\n" + bad) _, _, err := allowedRoots("/home/u") check(t, "invalid root "+bad, true, strings.Contains(fmt.Sprint(err), "allowed-paths")) } + write("/var") + _, err := checkLaunchDir("/var/tmp/x", "/nowhere/hosts") + check(t, "/var/tmp under /var root", true, err != nil) } func TestLaunchPassesArgsVerbatim(t *testing.T) {