Skip to content

Commit

Permalink
fixup! initial safe MkdirAll implementation
Browse files Browse the repository at this point in the history
Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Jun 26, 2024
1 parent 9766756 commit fd6804a
Showing 1 changed file with 24 additions and 18 deletions.
42 changes: 24 additions & 18 deletions mkdir_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
func partialOpen(root, unsafePath string) (_ *os.File, _ string, Err error) {
unsafePath = filepath.ToSlash(unsafePath) // noop

// Make sure there are no trailing slashes for the root path.
root = strings.TrimRight(root, "/")

// Try to use openat2 if possible.
if hasOpenat2() {
return partialOpenat2(root, unsafePath)
Expand All @@ -35,12 +32,21 @@ func partialOpen(root, unsafePath string) (_ *os.File, _ string, Err error) {
// This is very similar to SecureJoin, except that we operate on the
// components using file descriptors. We then return the last component we
// managed open, along with the remaining path components not opened.
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_NOFOLLOW|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
if err != nil {
return nil, "", err
}
defer rootDir.Close()

// Get the "actual" root path from /proc/self/fd. This is necessary if the
// root is some magic-link like /proc/$pid/root, in which case we want to
// make sure when we do checkProcSelfFdPath that we are using the correct
// root path.
logicalRootPath, err := procSelfFdReadlink(rootDir)
if err != nil {
return nil, "", fmt.Errorf("get real root path: %w", err)
}

currentDir, err := dupFile(rootDir)
if err != nil {
return nil, "", fmt.Errorf("clone root fd: %w", err)
Expand Down Expand Up @@ -75,9 +81,9 @@ func partialOpen(root, unsafePath string) (_ *os.File, _ string, Err error) {
nextPath := path.Join("/", currentPath, part)
// If we hit the root, continue on without actually opening it.
if nextPath == "/" {
// At this point, currentDir must be the same as rootDir (there is
// only a single ".." component), so we can just update the logical
// path without changing the handle.
// At this point, currentDir must be the same as rootDir (we will
// only land here when currentPath is /), so we can just update the
// logical path without changing the handle.
currentPath = nextPath
continue
}
Expand All @@ -94,11 +100,11 @@ func partialOpen(root, unsafePath string) (_ *os.File, _ string, Err error) {
// the system.
if part == ".." {
// Make sure the root hasn't moved.
if err := checkProcSelfFdPath(root, rootDir); err != nil {
if err := checkProcSelfFdPath(logicalRootPath, rootDir); err != nil {
return nil, "", fmt.Errorf("root path moved during lookup: %w", err)
}
// Make sure the path is what we expect.
fullPath := root + nextPath
fullPath := logicalRootPath + nextPath
if err := checkProcSelfFdPath(fullPath, nextDir); err != nil {
return nil, "", fmt.Errorf("walking into .. had unexpected result: %w", err)
}
Expand Down Expand Up @@ -139,12 +145,12 @@ func partialOpen(root, unsafePath string) (_ *os.File, _ string, Err error) {
remainingPath = linkDest + "/" + remainingPath
// Absolute symlinks reset any work we've already done.
if path.IsAbs(linkDest) {
nextDir, err := dupFile(rootDir)
rootDirCopy, err := dupFile(rootDir)
if err != nil {
return nil, "", fmt.Errorf("clone root fd: %w", err)
}
_ = currentDir.Close()
currentDir = nextDir
currentDir = rootDirCopy
currentPath = "/"
}

Expand Down Expand Up @@ -184,17 +190,17 @@ func MkdirAll(root, unsafePath string, mode os.FileMode) error {
// exist and thus there are no symlinks to worry about (and removing ..
// also removes the possibility of a parallel-write attack tricking us).
remainingPath = path.Join("/", filepath.ToSlash(remainingPath))
// Get rid of any trailing or leading / components so we don't get empty
// parts when we split the path.
remainingPath = strings.Trim(remainingPath, "/")

// Create the remaining components.
for _, part := range strings.Split(remainingPath, "/") {
switch part {
case "", ".":
// Skip no-op components. There should be none, but check anyway.
continue
case "..":
// This should never happen, but make sure we never walk up the
// tree.
return fmt.Errorf("[internal error] remaining path contained .. component")
case "", ".", "..":
// This should never happen, but especially in the case of .., make
// sure we don't hit any of these special parts.
return fmt.Errorf("[internal error] remaining path contained unexpected component %q", part)
}

// NOTE: mkdir(2) will not follow trailing symlinks, so we can safely
Expand Down

0 comments on commit fd6804a

Please sign in to comment.