Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"syscall" // only for SysProcAttr and Signal
"time"

"github.com/cyphar/filepath-securejoin"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/intelrdt"
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func (c *linuxContainer) makeCriuRestoreMountpoints(m *configs.Mount) error {
if err != nil {
return err
}
if err := checkMountDestination(c.config.Rootfs, dest); err != nil {
if err := checkProcMount(c.config.Rootfs, dest, ""); err != nil {
return err
}
m.Destination = dest
Expand Down
50 changes: 37 additions & 13 deletions libcontainer/rootfs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"strings"
"time"

"github.com/cyphar/filepath-securejoin"
securejoin "github.com/cyphar/filepath-securejoin"
"github.com/mrunalp/fileutils"
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
Expand Down Expand Up @@ -197,7 +197,7 @@ func prepareBindMount(m *configs.Mount, rootfs string) error {
if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
return err
}
if err := checkMountDestination(rootfs, dest); err != nil {
if err := checkProcMount(rootfs, dest, m.Source); err != nil {
return err
}
// update the mount with the correct dest after symlinks are resolved.
Expand Down Expand Up @@ -414,7 +414,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string, enableCgroupns b
if dest, err = securejoin.SecureJoin(rootfs, m.Destination); err != nil {
return err
}
if err := checkMountDestination(rootfs, dest); err != nil {
if err := checkProcMount(rootfs, dest, m.Source); err != nil {
return err
}
// update the mount with the correct dest after symlinks are resolved.
Expand Down Expand Up @@ -461,12 +461,12 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
return binds, nil
}

// checkMountDestination checks to ensure that the mount destination is not over the top of /proc.
// checkProcMount checks to ensure that the mount destination is not over the top of /proc.
// dest is required to be an abs path and have any symlinks resolved before calling this function.
func checkMountDestination(rootfs, dest string) error {
invalidDestinations := []string{
"/proc",
}
//
// if source is nil, don't stat the filesystem. This is used for restore of a checkpoint.
func checkProcMount(rootfs, dest, source string) error {
const procPath = "/proc"
// White list, it should be sub directories of invalid destinations
validDestinations := []string{
// These entries can be bind mounted by files emulated by fuse,
Expand All @@ -489,16 +489,40 @@ func checkMountDestination(rootfs, dest string) error {
return nil
}
}
for _, invalid := range invalidDestinations {
path, err := filepath.Rel(filepath.Join(rootfs, invalid), dest)
path, err := filepath.Rel(filepath.Join(rootfs, procPath), dest)
if err != nil {
return err
}
// pass if the mount path is located outside of /proc
if strings.HasPrefix(path, "..") {
return nil
}
if path == "." {
// an empty source is pasted on restore
if source == "" {
return nil
}
// only allow a mount on-top of proc if it's source is "proc"
isproc, err := isProc(source)
if err != nil {
return err
}
if path != "." && !strings.HasPrefix(path, "..") {
return fmt.Errorf("%q cannot be mounted because it is located inside %q", dest, invalid)
// pass if the mount is happening on top of /proc and the source of
// the mount is a proc filesystem
if isproc {
return nil
}
return fmt.Errorf("%q cannot be mounted because it is not of type proc", dest)
}
return nil
return fmt.Errorf("%q cannot be mounted because it is inside /proc", dest)
}

func isProc(path string) (bool, error) {
var s unix.Statfs_t
if err := unix.Statfs(path, &s); err != nil {
return false, err
}
return s.Type == unix.PROC_SUPER_MAGIC, nil
}

func setupDevSymlinks(rootfs string) error {
Expand Down
8 changes: 4 additions & 4 deletions libcontainer/rootfs_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,31 @@ import (

func TestCheckMountDestOnProc(t *testing.T) {
dest := "/rootfs/proc/sys"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err == nil {
t.Fatal("destination inside proc should return an error")
}
}

func TestCheckMountDestOnProcChroot(t *testing.T) {
dest := "/rootfs/proc/"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "/proc")
if err != nil {
t.Fatal("destination inside proc when using chroot should not return an error")
}
}

func TestCheckMountDestInSys(t *testing.T) {
dest := "/rootfs//sys/fs/cgroup"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err != nil {
t.Fatal("destination inside /sys should not return an error")
}
}

func TestCheckMountDestFalsePositive(t *testing.T) {
dest := "/rootfs/sysfiles/fs/cgroup"
err := checkMountDestination("/rootfs", dest)
err := checkProcMount("/rootfs", dest, "")
if err != nil {
t.Fatal(err)
}
Expand Down