From fd6804ab4bb489bbbeba13c0587ca57109d61cec Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <cyphar@cyphar.com>
Date: Thu, 27 Jun 2024 02:23:05 +1000
Subject: [PATCH] fixup! initial safe MkdirAll implementation

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
 mkdir_linux.go | 42 ++++++++++++++++++++++++------------------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/mkdir_linux.go b/mkdir_linux.go
index 8359696..bd09969 100644
--- a/mkdir_linux.go
+++ b/mkdir_linux.go
@@ -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)
@@ -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)
@@ -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
 		}
@@ -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)
 				}
@@ -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 = "/"
 			}
 
@@ -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