diff options
| author | Damien Neil <dneil@google.com> | 2025-04-16 11:01:19 -0700 |
|---|---|---|
| committer | Cherry Mui <cherryyz@google.com> | 2025-05-06 11:33:18 -0700 |
| commit | 92e23b683f01fe581a0e14b5658f0c59d9ce0d38 (patch) | |
| tree | f9b7ef365062d2777aff1a142557a477077d17d4 /src/os/root_openat.go | |
| parent | 30b2b767d6d902787b90476fd00eee4c9b3a3f15 (diff) | |
| download | go-92e23b683f01fe581a0e14b5658f0c59d9ce0d38.tar.xz | |
os: avoid escape from Root via paths ending in ../
The doInRoot function operates on a path split into components.
The final path component retained any trailing path separator
characters, to permit operations in a Root to retain the
trailing-separator behavior of non-Root operations. However,
doInRoot failed to take trailing separators into account
when checking for .. path components.
This could permit opening the parent directory of the Root
with a path ending in "../".
Change the split path to never include path separators in
components, and handle trailing separators independently
of the split path.
Thanks to Dan Sebastian Thrane of SDU eScience Center for
reporting this issue.
Fixes #73555
Fixes CVE-2025-22873
Change-Id: I9a33a145c22f5eb1dd4e4cafae5fcc61a8d4f0d4
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2160
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/670036
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Diffstat (limited to 'src/os/root_openat.go')
| -rw-r--r-- | src/os/root_openat.go | 17 |
1 files changed, 14 insertions, 3 deletions
diff --git a/src/os/root_openat.go b/src/os/root_openat.go index f67794cd72..db2ae2295f 100644 --- a/src/os/root_openat.go +++ b/src/os/root_openat.go @@ -178,7 +178,7 @@ func doInRoot[T any](r *Root, name string, f func(parent sysfdType, name string) } defer r.root.decref() - parts, err := splitPathInRoot(name, nil, nil) + parts, suffixSep, err := splitPathInRoot(name, nil, nil) if err != nil { return ret, err } @@ -242,7 +242,9 @@ func doInRoot[T any](r *Root, name string, f func(parent sysfdType, name string) // Call f to decide what to do with it. // If f returns errSymlink, this element is a symlink // which should be followed. - ret, err = f(dirfd, parts[i]) + // suffixSep contains any trailing separator characters + // which we rejoin to the final part at this time. + ret, err = f(dirfd, parts[i]+suffixSep) if _, ok := err.(errSymlink); !ok { return ret, err } @@ -264,10 +266,19 @@ func doInRoot[T any](r *Root, name string, f func(parent sysfdType, name string) if symlinks > rootMaxSymlinks { return ret, syscall.ELOOP } - newparts, err := splitPathInRoot(string(e), parts[:i], parts[i+1:]) + newparts, newSuffixSep, err := splitPathInRoot(string(e), parts[:i], parts[i+1:]) if err != nil { return ret, err } + if i == len(parts)-1 { + // suffixSep contains any trailing path separator characters + // in the link target. + // If we are replacing the remainder of the path, retain these. + // If we're replacing some intermediate component of the path, + // ignore them, since intermediate components must always be + // directories. + suffixSep = newSuffixSep + } if len(newparts) < i || !slices.Equal(parts[:i], newparts[:i]) { // Some component in the path which we have already traversed // has changed. We need to restart parsing from the root. |
