WIP: port SecureJoin to partialLookupInRoot

Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

join.go renamed to join_generic.go

@@ -29,23 +29,7 @@ func IsNotExist(err error) bool {
 	return errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) || errors.Is(err, syscall.ENOENT)
 }
 
-// SecureJoinVFS joins the two given path components (similar to Join) except
-// that the returned path is guaranteed to be scoped inside the provided root
-// path (when evaluated). Any symbolic links in the path are evaluated with the
-// given root treated as the root of the filesystem, similar to a chroot. The
-// filesystem state is evaluated through the given VFS interface (if nil, the
-// standard os.* family of functions are used).
-//
-// Note that the guarantees provided by this function only apply if the path
-// components in the returned string are not modified (in other words are not
-// replaced with symlinks on the filesystem) after this function has returned.
-// Such a symlink race is necessarily out-of-scope of SecureJoin.
-//
-// Volume names in unsafePath are always discarded, regardless if they are
-// provided via direct input or when evaluating symlinks. Therefore:
-//
-// "C:\Temp" + "D:\path\to\file.txt" results in "C:\Temp\path\to\file.txt"
-func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
+func legacySecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
 	// Use the os.* VFS implementation if none was specified.
 	if vfs == nil {
 		vfs = osVFS{}
@@ -116,9 +100,3 @@ func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
 	finalPath := filepath.Join(string(filepath.Separator), currentPath)
 	return filepath.Join(root, finalPath), nil
 }
-
-// SecureJoin is a wrapper around SecureJoinVFS that just uses the os.* library
-// of functions as the VFS. If in doubt, use this function over SecureJoinVFS.
-func SecureJoin(root, unsafePath string) (string, error) {
-	return SecureJoinVFS(root, unsafePath, nil)
-}

join_linux.go

@@ -0,0 +1,81 @@
+//go:build linux
+
+// Copyright (C) 2024 SUSE LLC. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package securejoin
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"golang.org/x/sys/unix"
+)
+
+func isLexicallyInRoot(root, path string) bool {
+	if root != "/" {
+		root += "/"
+	}
+	if path != "/" {
+		path += "/"
+	}
+	return strings.HasPrefix(path, root)
+}
+
+// SecureJoin is a wrapper around SecureJoinVFS that just uses the os.* library
+// of functions as the VFS. If in doubt, use this function over SecureJoinVFS.
+func SecureJoin(root, unsafePath string) (string, error) {
+	rootDir, err := os.OpenFile(root, unix.O_PATH|unix.O_DIRECTORY|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return "", err
+	}
+	defer rootDir.Close()
+
+	handle, remainingPath, err := partialLookupInRoot(rootDir, unsafePath, true)
+	if err != nil {
+		return "", err
+	}
+	defer handle.Close()
+
+	handlePath, err := procSelfFdReadlink(handle)
+	if err != nil {
+		return "", fmt.Errorf("verify actual path of %q handle: %w", handle.Name(), err)
+	}
+	// Make sure the path is inside the root.
+	if !isLexicallyInRoot(root, handlePath) {
+		return "", fmt.Errorf("%w: handle path %q is outside root %q", errPossibleBreakout, handlePath, root)
+	}
+
+	// remainingPath should be cleaned and safe to append, due to how
+	// unsafeHallucinateDirectories works. But do an additional cleanup, just
+	// to be sure.
+	remainingPath = filepath.Join("/", remainingPath)
+	return filepath.Join(handlePath, remainingPath), nil
+}
+
+// SecureJoinVFS joins the two given path components (similar to Join) except
+// that the returned path is guaranteed to be scoped inside the provided root
+// path (when evaluated). Any symbolic links in the path are evaluated with the
+// given root treated as the root of the filesystem, similar to a chroot. The
+// filesystem state is evaluated through the given VFS interface (if nil, the
+// standard os.* family of functions are used).
+//
+// Note that the guarantees provided by this function only apply if the path
+// components in the returned string are not modified (in other words are not
+// replaced with symlinks on the filesystem) after this function has returned.
+// Such a symlink race is necessarily out-of-scope of SecureJoin.
+//
+// Volume names in unsafePath are always discarded, regardless if they are
+// provided via direct input or when evaluating symlinks. Therefore:
+//
+// "C:\Temp" + "D:\path\to\file.txt" results in "C:\Temp\path\to\file.txt"
+func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
+	if vfs == nil || vfs == (osVFS{}) {
+		return SecureJoin(root, unsafePath)
+	}
+	// TODO: Make it possible for partialLookupInRoot to work with VFS.
+	return legacySecureJoinVFS(root, unsafePath, vfs)
+}

join_nonlinux.go

@@ -0,0 +1,33 @@
+//go:build !linux
+
+// Copyright (C) 2024 SUSE LLC. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package securejoin
+
+// SecureJoin is a wrapper around SecureJoinVFS that just uses the os.* library
+// of functions as the VFS. If in doubt, use this function over SecureJoinVFS.
+func SecureJoin(root, unsafePath string) (string, error) {
+	return SecureJoinVFS(root, unsafePath, nil)
+}
+
+// SecureJoinVFS joins the two given path components (similar to Join) except
+// that the returned path is guaranteed to be scoped inside the provided root
+// path (when evaluated). Any symbolic links in the path are evaluated with the
+// given root treated as the root of the filesystem, similar to a chroot. The
+// filesystem state is evaluated through the given VFS interface (if nil, the
+// standard os.* family of functions are used).
+//
+// Note that the guarantees provided by this function only apply if the path
+// components in the returned string are not modified (in other words are not
+// replaced with symlinks on the filesystem) after this function has returned.
+// Such a symlink race is necessarily out-of-scope of SecureJoin.
+//
+// Volume names in unsafePath are always discarded, regardless if they are
+// provided via direct input or when evaluating symlinks. Therefore:
+//
+// "C:\Temp" + "D:\path\to\file.txt" results in "C:\Temp\path\to\file.txt"
+func SecureJoinVFS(root, unsafePath string, vfs VFS) (string, error) {
+	return legacySecureJoinVFS(root, unsafePath, vfs)
+}

join_test.go

@@ -32,7 +32,7 @@ type input struct {
 }
 
 // Test basic handling of symlink expansion.
-func TestSymlink(t *testing.T) {
+func testSymlink(t *testing.T) {
 	dir := t.TempDir()
 	dir, err := filepath.EvalSymlinks(dir)
 	require.NoError(t, err)
@@ -77,6 +77,10 @@ func TestSymlink(t *testing.T) {
 	}
 }
 
+func TestSymlink(t *testing.T) {
+	withWithoutOpenat2(t, testSymlink)
+}
+
 // In a path without symlinks, SecureJoin is equivalent to Clean+Join.
 func TestNoSymlink(t *testing.T) {
 	dir := t.TempDir()

lookup_linux.go

@@ -116,11 +116,23 @@ func (s *symlinkStack) PopLastSymlink() (*os.File, string, bool) {
 	return tailEntry.dir, tailEntry.remainingPath, true
 }
 
+const maxUnsafeHallucinateDirectoryTries = 20
+
+var errTooManyFakeDirectories = errors.New("encountered too many non-existent paths")
+
 // partialLookupInRoot tries to lookup as much of the request path as possible
 // within the provided root (a-la RESOLVE_IN_ROOT) and opens the final existing
 // component of the requested path, returning a file handle to the final
 // existing component and a string containing the remaining path components.
-func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string, Err error) {
+//
+// If unsafeHallucinateDirectories is true, partialLookupInRoot will try to
+// emulate the legacy SecureJoin behaviour of treating non-existent paths as
+// though they are directories to try to resolve as much of the path as
+// possible. In practice, this means that a path like "a/b/doesnotexist/../c"
+// will end up being resolved as "a/b/c" if possible. Note that dangling
+// symlinks (a symlink that points to a non-existent path) will still result in
+// an error being returned, due to how openat2 handles symlinks.
+func partialLookupInRoot(root *os.File, unsafePath string, unsafeHallucinateDirectories bool) (_ *os.File, _ string, Err error) {
 	unsafePath = filepath.ToSlash(unsafePath) // noop
 
 	// This is very similar to SecureJoin, except that we operate on the
@@ -129,7 +141,7 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 
 	// Try to use openat2 if possible.
 	if hasOpenat2() {
-		return partialLookupOpenat2(root, unsafePath)
+		return partialLookupOpenat2(root, unsafePath, unsafeHallucinateDirectories)
 	}
 
 	// Get the "actual" root path from /proc/self/fd. This is necessary if the
@@ -168,7 +180,9 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 	defer symlinkStack.Close()
 
 	var (
-		linksWalked   int
+		linksWalked               int
+		hallucinateDirectoryTries int
+
 		currentPath   string
 		remainingPath = unsafePath
 	)
@@ -235,7 +249,12 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 					return nil, "", fmt.Errorf("root path moved during lookup: %w", err)
 				}
 				// Make sure the path is what we expect.
-				fullPath := logicalRootPath + nextPath
+				var fullPath string
+				if logicalRootPath == "/" {
+					fullPath = nextPath
+				} else {
+					fullPath = logicalRootPath + nextPath
+				}
 				if err := checkProcSelfFdPath(fullPath, nextDir); err != nil {
 					return nil, "", fmt.Errorf("walking into .. had unexpected result: %w", err)
 				}
@@ -250,6 +269,21 @@ func partialLookupInRoot(root *os.File, unsafePath string) (_ *os.File, _ string
 				_ = currentDir.Close()
 				return oldDir, remainingPath, nil
 			}
+			// If we were asked to "hallucinate" non-existent paths as though
+			// they are directories, take the remainingPath and clean it so
+			// that any ".." components that would lead us back to real paths
+			// can get resolved.
+			if oldRemainingPath != "" && unsafeHallucinateDirectories {
+				if newRemainingPath := path.Clean(oldRemainingPath); newRemainingPath != oldRemainingPath {
+					hallucinateDirectoryTries++
+					if hallucinateDirectoryTries > maxUnsafeHallucinateDirectoryTries {
+						return nil, "", fmt.Errorf("%w: trying to reconcile non-existent subpath %q", errTooManyFakeDirectories, oldRemainingPath)
+					}
+					// Continue the lookup using the new remaining path.
+					remainingPath = newRemainingPath
+					continue
+				}
+			}
 			// We have hit a final component that doesn't exist, so we have our
 			// partial open result. Note that we have to use the OLD remaining
 			// path, since the lookup failed.

mkdir_linux.go

@@ -40,7 +40,7 @@ var errPossibleAttack = errors.New("possible attack detected")
 // should use MkdirAllHandle.
 func MkdirAllHandle(root *os.File, unsafePath string, mode os.FileMode) (_ *os.File, Err error) {
 	// Try to open as much of the path as possible.
-	currentDir, remainingPath, err := partialLookupInRoot(root, unsafePath)
+	currentDir, remainingPath, err := partialLookupInRoot(root, unsafePath, false)
 	if err != nil {
 		return nil, fmt.Errorf("find existing subpath of %q: %w", unsafePath, err)
 	}

mkdir_linux_test.go

@@ -95,7 +95,7 @@ func testMkdirAllBasic(t *testing.T, mkdirAll func(t *testing.T, root, unsafePat
 
 				// Before trying to make the tree, figure out what
 				// components don't exist yet so we can check them later.
-				handle, remainingPath, err := partialLookupInRoot(rootDir, test.unsafePath)
+				handle, remainingPath, err := partialLookupInRoot(rootDir, test.unsafePath, false)
 				handleName := "<nil>"
 				if handle != nil {
 					handleName = handle.Name()

openat2_linux.go

@@ -85,14 +85,16 @@ func openat2File(dir *os.File, path string, how *unix.OpenHow) (*os.File, error)
 // partialLookupOpenat2 is an alternative implementation of
 // partialLookupInRoot, using openat2(RESOLVE_IN_ROOT) to more safely get a
 // handle to the deepest existing child of the requested path within the root.
-func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, error) {
+func partialLookupOpenat2(root *os.File, unsafePath string, unsafeHallucinateDirectories bool) (*os.File, string, error) {
+	unsafePath = filepath.ToSlash(unsafePath) // noop
+
 	if !hasOpenat2() {
 		return nil, "", fmt.Errorf("openat2: %w", unix.ENOTSUP)
 	}
 
 	// TODO: Implement this as a git-bisect-like binary search.
 
-	unsafePath = filepath.ToSlash(unsafePath) // noop
+	var hallucinateDirectoryTries int
 	endIdx := len(unsafePath)
 	for endIdx > 0 {
 		subpath := unsafePath[:endIdx]
@@ -104,7 +106,25 @@ func partialLookupOpenat2(root *os.File, unsafePath string) (*os.File, string, e
 		})
 		if err == nil {
 			// We found a subpath!
-			return handle, unsafePath[endIdx:], nil
+			remainingPath := unsafePath[endIdx:]
+			// If we were asked to "hallucinate" non-existent paths as though
+			// they are directories, take the remainingPath and clean it so
+			// that any ".." components that would lead us back to real paths
+			// can get resolved.
+			if remainingPath != "" && unsafeHallucinateDirectories {
+				if newRemainingPath := filepath.Clean(remainingPath); newRemainingPath != remainingPath {
+					hallucinateDirectoryTries++
+					if hallucinateDirectoryTries > maxUnsafeHallucinateDirectoryTries {
+						return nil, "", fmt.Errorf("%w: trying to reconcile non-existent subpath %q", errTooManyFakeDirectories, remainingPath)
+					}
+					// Start the lookup from the end again using the new
+					// remaining path.
+					unsafePath = subpath + "/" + newRemainingPath
+					endIdx = len(unsafePath)
+					continue
+				}
+			}
+			return handle, remainingPath, nil
 		}
 		if !errors.Is(err, os.ErrNotExist) {
 			return nil, "", fmt.Errorf("open subpath: %w", err)