diff --git a/internal/pathrs/mkdirall_pathrslite.go b/internal/pathrs/mkdirall_pathrslite.go index 0661273b1..c2578e051 100644 --- a/internal/pathrs/mkdirall_pathrslite.go +++ b/internal/pathrs/mkdirall_pathrslite.go @@ -21,7 +21,6 @@ package pathrs import ( "fmt" "os" - "path/filepath" "github.com/cyphar/filepath-securejoin/pathrs-lite" "github.com/sirupsen/logrus" @@ -50,18 +49,9 @@ import ( // needed for a lot of runc callers and fixing this would require reworking a // lot of path logic). func MkdirAllInRoot(root, unsafePath string, mode os.FileMode) (*os.File, error) { - // If the path is already "within" the root, get the path relative to the - // root and use that as the unsafe path. This is necessary because a lot of - // MkdirAllInRoot callers have already done SecureJoin, and refactoring - // all of them to stop using these SecureJoin'd paths would require a fair - // amount of work. - // TODO(cyphar): Do the refactor to libpathrs once it's ready. - if IsLexicallyInRoot(root, unsafePath) { - subPath, err := filepath.Rel(root, unsafePath) - if err != nil { - return nil, err - } - unsafePath = subPath + unsafePath, err := hallucinateUnsafePath(root, unsafePath) + if err != nil { + return nil, fmt.Errorf("failed to construct hallucinated target path: %w", err) } // Check for any silly mode bits. diff --git a/internal/pathrs/path.go b/internal/pathrs/path.go index 709c3ae3f..77be98924 100644 --- a/internal/pathrs/path.go +++ b/internal/pathrs/path.go @@ -22,6 +22,8 @@ import ( "os" "path/filepath" "strings" + + securejoin "github.com/cyphar/filepath-securejoin" ) // IsLexicallyInRoot is shorthand for strings.HasPrefix(path+"/", root+"/"), @@ -82,3 +84,33 @@ func LexicallyStripRoot(root, path string) string { } return LexicallyCleanPath("/" + path) } + +// hallucinateUnsafePath creates a new unsafePath which has all symlinks +// (including dangling symlinks) fully resolved and any non-existent components +// treated as though they are real. This is effectively just a wrapper around +// [securejoin.SecureJoin] that strips the root. This path *IS NOT* safe to use +// as-is, you *MUST* operate on the returned path with pathrs-lite. +// +// The reason for this methods is that in previous runc versions, we would +// tolerate nonsense paths with dangling symlinks as path components. +// pathrs-lite does not support this, so instead we have to emulate this +// behaviour by doing SecureJoin *purely to get a semi-reasonable path to use* +// and then we use pathrs-lite to operate on the path safely. +// +// It would be quite difficult to emulate this in a race-free way in +// pathrs-lite, so instead we use [securejoin.SecureJoin] to simply produce a +// new candidate path for operations like [MkdirAllInRoot] so they can then +// operate on the new unsafePath as if it was what the user requested. +// +// If unsafePath is already lexically inside root, it is stripped before +// re-resolving it (this is done to ensure compatibility with legacy callers +// within runc that call SecureJoin before calling into pathrs). +func hallucinateUnsafePath(root, unsafePath string) (string, error) { + unsafePath = LexicallyStripRoot(root, unsafePath) + weirdPath, err := securejoin.SecureJoin(root, unsafePath) + if err != nil { + return "", err + } + unsafePath = LexicallyStripRoot(root, weirdPath) + return unsafePath, nil +} diff --git a/internal/pathrs/root_pathrslite.go b/internal/pathrs/root_pathrslite.go index e90ac3114..051f9236b 100644 --- a/internal/pathrs/root_pathrslite.go +++ b/internal/pathrs/root_pathrslite.go @@ -48,6 +48,11 @@ func OpenInRoot(root, subpath string, flags int) (*os.File, error) { // include it in the passed flags. The fileMode argument uses unix.* mode bits, // *not* os.FileMode. func CreateInRoot(root, subpath string, flags int, fileMode uint32) (*os.File, error) { + subpath, err := hallucinateUnsafePath(root, subpath) + if err != nil { + return nil, fmt.Errorf("failed to construct hallucinated target path: %w", err) + } + dir, filename := filepath.Split(subpath) if filepath.Join("/", filename) == "/" { return nil, fmt.Errorf("create in root subpath %q has bad trailing component %q", subpath, filename) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index deb209c18..e05a0dc53 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -545,18 +545,6 @@ func (m *mountEntry) createOpenMountpoint(rootfs string) (Err error) { } dstIsFile = !fi.IsDir() } - - // In previous runc versions, we would tolerate nonsense paths with - // dangling symlinks as path components. pathrs-lite does not support - // this, so instead we have to emulate this behaviour by doing - // SecureJoin *purely to get a semi-reasonable path to use* and then we - // use pathrs-lite to operate on the path safely. - newUnsafePath, err := securejoin.SecureJoin(rootfs, unsafePath) - if err != nil { - return err - } - unsafePath = pathrs.LexicallyStripRoot(rootfs, newUnsafePath) - if dstIsFile { dstFile, err = pathrs.CreateInRoot(rootfs, unsafePath, unix.O_CREAT|unix.O_EXCL|unix.O_NOFOLLOW, 0o644) } else {