WIP: port SecureJoin to partialLookupInRoot

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Author: cyphar

join.go join_generic.gojoin.go renamed to join_generic.go

@@ -29,29 +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.
-//
-// NOTE: Due to the above limitation, Linux users are strongly encouraged to
-// use OpenInRoot instead, which does safely protect against these kinds of
-// attacks. There is no way to solve this problem with SecureJoinVFS because
-// the API is fundamentally wrong (you cannot return a "safe" path string and
-// guarantee it won't be modified afterwards).
-//
-// 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{}
@@ -122,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_linux_test.go

@@ -0,0 +1,13 @@
+// Copyright (C) 2017-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 (
+	"testing"
+)
+
+func TestSymlink(t *testing.T) {
+	withWithoutOpenat2(t, true, testSymlink)
+}

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)

lookup_linux.go

@@ -153,11 +153,23 @@ func (s *symlinkStack) PopTopSymlink() (*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
@@ -166,7 +178,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
@@ -204,7 +216,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
 	)
@@ -354,6 +368,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.

lookup_linux_test.go

@@ -19,7 +19,7 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-type partialLookupFunc func(root *os.File, unsafePath string) (*os.File, string, error)
+type partialLookupFunc func(root *os.File, unsafePath string, hallucinateDirectoryTries bool) (*os.File, string, error)
 
 type lookupResult struct {
 	handlePath, remainingPath string
@@ -28,7 +28,7 @@ type lookupResult struct {
 }
 
 func checkPartialLookup(t *testing.T, partialLookupFn partialLookupFunc, rootDir *os.File, unsafePath string, expected lookupResult) {
-	handle, remainingPath, err := partialLookupFn(rootDir, unsafePath)
+	handle, remainingPath, err := partialLookupFn(rootDir, unsafePath, false)
 	if handle != nil {
 		defer handle.Close()
 	}
@@ -325,7 +325,7 @@ func newRacingLookupMeta(pauseCh chan struct{}) *racingLookupMeta {
 func (m *racingLookupMeta) checkPartialLookup(t *testing.T, rootDir *os.File, unsafePath string, skipErrs []error, allowedResults []lookupResult) {
 	// Similar to checkPartialLookup, but with extra logic for
 	// handling the lookup stopping partly through the lookup.
-	handle, remainingPath, err := partialLookupInRoot(rootDir, unsafePath)
+	handle, remainingPath, err := partialLookupInRoot(rootDir, unsafePath, false)
 	if err != nil {
 		for _, skipErr := range skipErrs {
 			if errors.Is(err, skipErr) {

mkdir_linux.go

@@ -48,7 +48,7 @@ func MkdirAllHandle(root *os.File, unsafePath string, mode int) (_ *os.File, Err
 	}
 
 	// 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

@@ -110,7 +110,7 @@ func testMkdirAll_Basic(t *testing.T, mkdirAll func(t *testing.T, root, unsafePa
 
 				// 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()

open_linux.go

@@ -16,7 +16,7 @@ import (
 // OpenatInRoot is equivalent to OpenInRoot, except that the root is provided
 // using an *os.File handle, to ensure that the correct root directory is used.
 func OpenatInRoot(root *os.File, unsafePath string) (*os.File, error) {
-	handle, remainingPath, err := partialLookupInRoot(root, unsafePath)
+	handle, remainingPath, err := partialLookupInRoot(root, unsafePath, false)
 	if err != nil {
 		return nil, err
 	}