diff options
| author | Olivier Mengué <olivier.mengue@gmail.com> | 2025-06-30 16:58:59 +0200 |
|---|---|---|
| committer | Mark Freeman <mark@golang.org> | 2025-07-30 13:35:04 -0700 |
| commit | 8fa31a2d7d9e60c50a3a94080c097b6e65773f4b (patch) | |
| tree | f59156d76a368fad09eb16b8cccdfff6bbf3adea | |
| parent | e8794e650e05fad07a33fb6e3266a9e677d13fa8 (diff) | |
| download | go-8fa31a2d7d9e60c50a3a94080c097b6e65773f4b.tar.xz | |
[release-branch.go1.23] os/exec: fix incorrect expansion of "", "." and ".." in LookPath
Fix incorrect expansion of "" and "." when $PATH contains an executable
file or, on Windows, a parent directory of a %PATH% element contains an
file with the same name as the %PATH% element but with one of the
%PATHEXT% extension (ex: C:\utils\bin is in PATH, and C:\utils\bin.exe
exists).
Fix incorrect expansion of ".." when $PATH contains an element which is
an the concatenation of the path to an executable file (or on Windows
a path that can be expanded to an executable by appending a %PATHEXT%
extension), a path separator and a name.
"", "." and ".." are now rejected early with ErrNotFound.
Fixes CVE-2025-47906
Fixes #74803
Change-Id: Ie50cc0a660fce8fbdc952a7f2e05c36062dcb50e
Reviewed-on: https://go-review.googlesource.com/c/go/+/685755
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
(cherry picked from commit e0b07dc22eaab1b003d98ad6d63cdfacc76c5c70)
Reviewed-on: https://go-review.googlesource.com/c/go/+/691855
Reviewed-by: Michael Knyszek <mknyszek@google.com>
| -rw-r--r-- | src/os/exec/dot_test.go | 56 | ||||
| -rw-r--r-- | src/os/exec/exec.go | 10 | ||||
| -rw-r--r-- | src/os/exec/lp_plan9.go | 4 | ||||
| -rw-r--r-- | src/os/exec/lp_unix.go | 4 | ||||
| -rw-r--r-- | src/os/exec/lp_windows.go | 8 |
5 files changed, 82 insertions, 0 deletions
diff --git a/src/os/exec/dot_test.go b/src/os/exec/dot_test.go index ed4bad23b1..86e9cbb4cf 100644 --- a/src/os/exec/dot_test.go +++ b/src/os/exec/dot_test.go @@ -178,4 +178,60 @@ func TestLookPath(t *testing.T) { } } }) + + checker := func(test string) func(t *testing.T) { + return func(t *testing.T) { + t.Helper() + t.Logf("PATH=%s", os.Getenv("PATH")) + p, err := LookPath(test) + if err == nil { + t.Errorf("%q: error expected, got nil", test) + } + if p != "" { + t.Errorf("%q: path returned should be \"\". Got %q", test, p) + } + } + } + + // Reference behavior for the next test + t.Run(pathVar+"=$OTHER2", func(t *testing.T) { + t.Run("empty", checker("")) + t.Run("dot", checker(".")) + t.Run("dotdot1", checker("abc/..")) + t.Run("dotdot2", checker("..")) + }) + + // Test the behavior when PATH contains an executable file which is not a directory + t.Run(pathVar+"=exe", func(t *testing.T) { + // Inject an executable file (not a directory) in PATH. + // Use our own binary os.Args[0]. + testenv.MustHaveExec(t) + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + + t.Setenv(pathVar, exe) + t.Run("empty", checker("")) + t.Run("dot", checker(".")) + t.Run("dotdot1", checker("abc/..")) + t.Run("dotdot2", checker("..")) + }) + + // Test the behavior when PATH contains an executable file which is not a directory + t.Run(pathVar+"=exe/xx", func(t *testing.T) { + // Inject an executable file (not a directory) in PATH. + // Use our own binary os.Args[0]. + testenv.MustHaveExec(t) + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + + t.Setenv(pathVar, filepath.Join(exe, "xx")) + t.Run("empty", checker("")) + t.Run("dot", checker(".")) + t.Run("dotdot1", checker("abc/..")) + t.Run("dotdot2", checker("..")) + }) } diff --git a/src/os/exec/exec.go b/src/os/exec/exec.go index da9f68fe28..a5e2dec24b 100644 --- a/src/os/exec/exec.go +++ b/src/os/exec/exec.go @@ -1310,3 +1310,13 @@ func addCriticalEnv(env []string) []string { // Code should use errors.Is(err, ErrDot), not err == ErrDot, // to test whether a returned error err is due to this condition. var ErrDot = errors.New("cannot run executable found relative to current directory") + +// validateLookPath excludes paths that can't be valid +// executable names. See issue #74466 and CVE-2025-47906. +func validateLookPath(s string) error { + switch s { + case "", ".", "..": + return ErrNotFound + } + return nil +} diff --git a/src/os/exec/lp_plan9.go b/src/os/exec/lp_plan9.go index 87359b3551..0430af9eef 100644 --- a/src/os/exec/lp_plan9.go +++ b/src/os/exec/lp_plan9.go @@ -36,6 +36,10 @@ func findExecutable(file string) error { // As of Go 1.19, LookPath will instead return that path along with an error satisfying // [errors.Is](err, [ErrDot]). See the package documentation for more details. func LookPath(file string) (string, error) { + if err := validateLookPath(file); err != nil { + return "", &Error{file, err} + } + // skip the path lookup for these prefixes skip := []string{"/", "#", "./", "../"} diff --git a/src/os/exec/lp_unix.go b/src/os/exec/lp_unix.go index 8617d45e98..e5fddbafe2 100644 --- a/src/os/exec/lp_unix.go +++ b/src/os/exec/lp_unix.go @@ -54,6 +54,10 @@ func LookPath(file string) (string, error) { // (only bypass the path if file begins with / or ./ or ../) // but that would not match all the Unix shells. + if err := validateLookPath(file); err != nil { + return "", &Error{file, err} + } + if strings.Contains(file, "/") { err := findExecutable(file) if err == nil { diff --git a/src/os/exec/lp_windows.go b/src/os/exec/lp_windows.go index 0e058d41b0..4b4e297112 100644 --- a/src/os/exec/lp_windows.go +++ b/src/os/exec/lp_windows.go @@ -68,6 +68,10 @@ func findExecutable(file string, exts []string) (string, error) { // As of Go 1.19, LookPath will instead return that path along with an error satisfying // [errors.Is](err, [ErrDot]). See the package documentation for more details. func LookPath(file string) (string, error) { + if err := validateLookPath(file); err != nil { + return "", &Error{file, err} + } + return lookPath(file, pathExt()) } @@ -81,6 +85,10 @@ func LookPath(file string) (string, error) { // "C:\foo\example.com" would be returned as-is even if the // program is actually "C:\foo\example.com.exe". func lookExtensions(path, dir string) (string, error) { + if err := validateLookPath(path); err != nil { + return "", &Error{path, err} + } + if filepath.Base(path) == path { path = "." + string(filepath.Separator) + path } |
