merge #54 into cyphar/filepath-securejoin:main

Aleksa Sarai (1):
  *: unify nil dir handling for *at(2) wrappers

LGTMs: cyphar

Author: cyphar

openat2_linux.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	"golang.org/x/sys/unix"
@@ -44,12 +45,12 @@ func scopedLookupShouldRetry(how *unix.OpenHow, err error) bool {
 const scopedLookupMaxRetries = 10
 
 func openat2File(dir *os.File, path string, how *unix.OpenHow) (*os.File, error) {
-	fullPath := dir.Name() + "/" + path
+	dirFd, fullPath := prepareAt(dir, path)
 	// Make sure we always set O_CLOEXEC.
 	how.Flags |= unix.O_CLOEXEC
 	var tries int
 	for tries < scopedLookupMaxRetries {
-		fd, err := unix.Openat2(int(dir.Fd()), path, how)
+		fd, err := unix.Openat2(dirFd, path, how)
 		if err != nil {
 			if scopedLookupShouldRetry(how, err) {
 				// We retry a couple of times to avoid the spurious errors, and
@@ -60,6 +61,7 @@ func openat2File(dir *os.File, path string, how *unix.OpenHow) (*os.File, error)
 			}
 			return nil, &os.PathError{Op: "openat2", Path: fullPath, Err: err}
 		}
+		runtime.KeepAlive(dir)
 		// If we are using RESOLVE_IN_ROOT, the name we generated may be wrong.
 		// NOTE: The procRoot code MUST NOT use RESOLVE_IN_ROOT, otherwise
 		//       you'll get infinite recursion here.

openat_linux.go

@@ -9,6 +9,7 @@ package securejoin
 import (
 	"os"
 	"path/filepath"
+	"runtime"
 
 	"golang.org/x/sys/unix"
 )
@@ -21,35 +22,64 @@ func dupFile(f *os.File) (*os.File, error) {
 	return os.NewFile(uintptr(fd), f.Name()), nil
 }
 
+// prepareAtWith returns -EBADF (an invalid fd) if dir is nil, otherwise using
+// the dir.Fd(). We use -EBADF because in filepath-securejoin we generally
+// don't want to allow relative-to-cwd paths. The returned path is an
+// *informational* string that describes a reasonable pathname for the given
+// *at(2) arguments. You must not use the full path for any actual filesystem
+// operations.
+func prepareAt(dir *os.File, path string) (dirFd int, unsafeFullPath string) {
+	dirFd, dirPath := -int(unix.EBADF), "."
+	if dir != nil {
+		dirFd, dirPath = int(dir.Fd()), dir.Name()
+	}
+	if !filepath.IsAbs(path) {
+		// only prepend the dirfd path for relative paths
+		path = dirPath + "/" + path
+	}
+	// NOTE: If path is "." or "", the returned path won't be filepath.Clean,
+	// but that's okay since this path is either used for errors (in which case
+	// a trailing "/" or "/." is important information) or will be
+	// filepath.Clean'd later (in the case of openatFile).
+	return dirFd, path
+}
+
 func openatFile(dir *os.File, path string, flags int, mode int) (*os.File, error) { //nolint:unparam // wrapper func
+	dirFd, fullPath := prepareAt(dir, path)
 	// Make sure we always set O_CLOEXEC.
 	flags |= unix.O_CLOEXEC
-	fd, err := unix.Openat(int(dir.Fd()), path, flags, uint32(mode))
+	fd, err := unix.Openat(dirFd, path, flags, uint32(mode))
 	if err != nil {
-		return nil, &os.PathError{Op: "openat", Path: dir.Name() + "/" + path, Err: err}
+		return nil, &os.PathError{Op: "openat", Path: fullPath, Err: err}
 	}
-	// All of the paths we use with openatFile(2) are guaranteed to be
-	// lexically safe, so we can use path.Join here.
-	fullPath := filepath.Join(dir.Name(), path)
+	runtime.KeepAlive(dir)
+	// openat is only used with lexically-safe paths so we can use
+	// filepath.Clean here, and also the path itself is not going to be used
+	// for actual path operations.
+	fullPath = filepath.Clean(fullPath)
 	return os.NewFile(uintptr(fd), fullPath), nil
 }
 
 func fstatatFile(dir *os.File, path string, flags int) (unix.Stat_t, error) {
+	dirFd, fullPath := prepareAt(dir, path)
 	var stat unix.Stat_t
-	if err := unix.Fstatat(int(dir.Fd()), path, &stat, flags); err != nil {
-		return stat, &os.PathError{Op: "fstatat", Path: dir.Name() + "/" + path, Err: err}
+	if err := unix.Fstatat(dirFd, path, &stat, flags); err != nil {
+		return stat, &os.PathError{Op: "fstatat", Path: fullPath, Err: err}
 	}
+	runtime.KeepAlive(dir)
 	return stat, nil
 }
 
 func readlinkatFile(dir *os.File, path string) (string, error) {
+	dirFd, fullPath := prepareAt(dir, path)
 	size := 4096
 	for {
 		linkBuf := make([]byte, size)
-		n, err := unix.Readlinkat(int(dir.Fd()), path, linkBuf)
+		n, err := unix.Readlinkat(dirFd, path, linkBuf)
 		if err != nil {
-			return "", &os.PathError{Op: "readlinkat", Path: dir.Name() + "/" + path, Err: err}
+			return "", &os.PathError{Op: "readlinkat", Path: fullPath, Err: err}
 		}
+		runtime.KeepAlive(dir)
 		if n != size {
 			return string(linkBuf[:n]), nil
 		}

procfs_linux.go

@@ -113,19 +113,15 @@ func newPrivateProcMount() (*os.File, error) {
 }
 
 func openTree(dir *os.File, path string, flags uint) (*os.File, error) {
-	dirFd := -int(unix.EBADF)
-	dirName := "."
-	if dir != nil {
-		dirFd = int(dir.Fd())
-		dirName = dir.Name()
-	}
+	dirFd, fullPath := prepareAt(dir, path)
 	// Make sure we always set O_CLOEXEC.
 	flags |= unix.OPEN_TREE_CLOEXEC
 	fd, err := unix.OpenTree(dirFd, path, flags)
 	if err != nil {
-		return nil, &os.PathError{Op: "open_tree", Path: path, Err: err}
+		return nil, &os.PathError{Op: "open_tree", Path: fullPath, Err: err}
 	}
-	return os.NewFile(uintptr(fd), dirName+"/"+path), nil
+	runtime.KeepAlive(dir)
+	return os.NewFile(uintptr(fd), fullPath), nil
 }
 
 func clonePrivateProcMount() (_ *os.File, Err error) {
@@ -325,15 +321,17 @@ func getMountID(dir *os.File, path string) (uint64, error) {
 		wantStxMask uint32 = STATX_MNT_ID_UNIQUE | unix.STATX_MNT_ID
 	)
 
-	err := unix.Statx(int(dir.Fd()), path, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
+	dirFd, fullPath := prepareAt(dir, path)
+	err := unix.Statx(dirFd, path, unix.AT_EMPTY_PATH|unix.AT_SYMLINK_NOFOLLOW, int(wantStxMask), &stx)
 	if stx.Mask&wantStxMask == 0 {
 		// It's not a kernel limitation, for some reason we couldn't get a
 		// mount ID. Assume it's some kind of attack.
 		err = fmt.Errorf("%w: could not get mount id", errUnsafeProcfs)
 	}
 	if err != nil {
-		return 0, &os.PathError{Op: "statx(STATX_MNT_ID_...)", Path: dir.Name() + "/" + path, Err: err}
+		return 0, &os.PathError{Op: "statx(STATX_MNT_ID_...)", Path: fullPath, Err: err}
 	}
+	runtime.KeepAlive(dir)
 	return stx.Mnt_id, nil
 }