os/exec: return error when PATH lookup would use current directory

We recently published a new package golang.org/x/sys/execabs, which is a forwarding wrapper around os/exec that makes three changes:

execabs.LookPath changes the result of exec.LookPath in the case where a PATH search returns an executable in the current directory (dot). In that case, execabs.LookPath returns an error instead. The error text is “prog resolves to executable in current directory (./prog)”
execabs.Command changes the result of exec.Command in the same case. It arranges that the subsequent cmd.Run or cmd.Start will return that same error.
execabs.CommandContext does the same.

As yet another possible answer in the “what to do about PATH lookups on Windows” saga, perhaps we should change os/exec to do these things by default. I am filing this proposal to help us discuss and decide whether to take this path. (For more background, see the blog post https://blog.golang.org/path-security.)

The proposal can be summarized as “replace os/exec with golang.org/x/sys/execabs”.

To recap how we got here, the fundamental problem is that lots of Go programs do things like exec.Command("prog") and expect that prog will come from a system directory listed in the PATH. It is a surprise and in some cases a security problem that on Windows prog will be taken from dot instead when prog.exe (or prog.bat, prog.com, ...) exists. The same is true on Unix for users who have an explicit dot or empty-string element in their PATH.

We already have three issues with possible approaches to solving this problem. They are:

proposal: os/exec: make LookPath not look in dot implicitly on Windows #38736, which removes the implicit dot lookup from LookPath on Windows
proposal: os/exec: add LookPathAbs that refuses to return relative paths #42420, to add LookPathAbs that doesn't return relative results.
proposal: os/exec: use LookPathAbs by default #42950, to make exec.Command use LookPathAbs by default (assuming it is added)

The problem with #38736 is that it silently changes the behavior of programs on Windows. Where exec.Command("prog") previously found and ran .\prog.exe, it would now either silently switch to a prog.exe from the PATH (surprise!) or return an error that prog could not be found (even though it used to be; confusion!). The same is true of exec.LookPath.

#42420 avoids the problem of changing existing behavior by introducing a new function exec.LookPathAbs that never looks in dot. Clearly that doesn’t surprise or confuse anyone. But it also doesn’t fix any security problems.

#42950 extends #42420 by changing exec.Command to use exec.LookPathAbs by default. That brings back the surprise and confusion of #38736, but only for exec.Command and not exec.LookPath. And compared to #38736, #42420+#42950 has the added complexity of adding new API (LookPathAbs).

None of these are great.

The proposal in this issue, to adopt execabs semantics as the os/exec semantics, fixes the problems. Execabs doesn’t remove dot from the PATH lookup. Instead it reports use of dot as an error. This avoids the surprise of running a different program. And the reported error is very clear about what happened. Instead of a generic “program not found” it gives an error that avoids the confusion:

prog resolves to executable in current directory (./prog)

And of course because programs from the current directory are not being executed, that fixes the security problem too.

The specific changes this issue proposes in os/exec are:

Add a new var ErrDot = errors.New("executable in current directory")
Change LookPath: if it would have chosen to return path, nil where path is an executable in the current directory found by a PATH lookup, it now does return path, err where err satisfies errors.Is(err, ErrDot).
Change Cmd: add a new field Err error which is returned by Start or Run if not set. This field replaces the current unexported field lookPathErr.

Consider this client code:

path, err := exec.LookPath("prog")
if err != nil {
	log.Fatal(err)
}
use(path)

cmd := exec.Command("prog")
if err := cmd.Run(); err != nil {
	log.Fatal(err)
}

With the proposed changes, the code would no longer find and run ./prog on Unix (when dot is in the PATH), nor .\prog.exe on Windows (regardless of PATH), addressing the security issue.

Programs that want to require the current directory to be used (ignoring PATH) can change "prog" to "./prog" (that works on both Unix and Windows systems). This change ("prog" to "./prog") is compatible with older versions of os/exec.

Programs that want to allow the use of the current directory in conjunction with PATH can add a few new lines of code, marked with <<<:

path, err := exec.LookPath("prog")
if errors.Is(err, exec.ErrDot) {        // <<<
	err = nil                       // <<<
}                                       // <<<
if err != nil {
	log.Fatal(err)
}
use(path)

cmd := exec.Command("prog")
if errors.Is(cmd.Err, exec.ErrDot) {    // <<<
	cmd.Err = nil                   // <<<
}                                       // <<<
if err := cmd.Run(); err != nil {
	log.Fatal(err)
}

This should give programs the flexibility to opt back into the old behavior when necessary.
Of course, this change (adding the errors.Is checks) is not compatible with older versions of os/exec, but we expect this need to be rare. We expect most programs that intentionally run programs from the current directory to update to the ./prog form.

Windows users, would this proposal break your programs?

You can check today by replacing

import "os/exec"

with

import exec "golang.org/x/sys/execabs"

in your source code. No other changes are needed to get the effect. (Of course, golang.org/x/sys/execabs does not provide ErrDot, so the errors.Is stanzas cannot be written in this simulation of the proposal.)

Thoughts? Comments? Concerns? Thanks very much.

As a part time Windows user, I don't think I expect exec.Command("prog") to work the same way as I expect launching stuff from cmd.exe. The cmd.exe behavior is surprising, but it's already set in stone several decades ago.

I don't understand why the Cmd.Err field is necessary. Couldn't the workaround program instead be written as

	const short = "prog"
	actual := short
	if abs, err := exec.LookPath(short); errors.Is(err, exec.ErrDot) {
		actual = abs
	}
	cmd := exec.Command(actual)
	cmd.Args[0] = short

?

@rsc

Thank you for the detailed description, @rsc!

If this proposal would be adopted, how should we write the following code so it would be safe to run inside an untrusted directory?

c := exec.Command("git", "status")
s, err := c.Output()

With this proposal, if the current directory has a .\git.exe or a .\git.bat (or a similar file matching any of the PATHEXTs), running this command would result in a failure. Since there might be legitimate reasons why the current directory could contain a git.bat, would we have any option to instead execute git as found in the system PATH instead of in the current directory?

I understand that this proposal is meant to prevent silent failures in programs that relied on the Windows behavior. I'm just wondering, if this proposal gets accepted, what are our options for running commands in untrusted directories (i.e. so that the contents of the current directory is entirely ignored)? Might this proposal ship in tandem with #42420?

For some additional context, I recently updated to Go 1.15.7 (which includes the security patch for this issue) and it broke build-time scripts that run on Mac and/or Linux systems. We have a build process where we intentionally go install build tools to <project-dir>/bin (in order to avoid installing local tools to "global" directories that happen to appear in $PATH) and prepend <project-dir>/bin to $PATH. We then have //go:generate custom-tool .... directives in .go source files within the project, with the obvious expectation that it will find the custom-tool binary that we just "installed" into <project-dir>/bin.

Unfortunately for me, running go generate ./... from the project root now fails with an error that says:

custom-tool resolves to executable in current directory (./bin/custom-tool)

The suggested workaround is to prepend ./ (or ./bin/ in my case) would work but it now makes those go:generate directives sensitive to which directory go generate is run from, i.e. a go:generate directive in a sub-package would work when go generate is run from the project root but fail if the command was run from the sub-package directory.

While I fully understand the security concern here, I take issue with the premise that all situations that involve executing a binary that resides within the current directory are invalid. With this change to Go, I now have approximately 800 go:generate directives that I need to investigate to see whether or not they are broken.

@dylan-bourque

@dylan-bourque wouldn't using the absolute path to the bin directory be a more proper fix for your situation?

# assuming this is set from tooling in the root of the project
export PATH=$(pwd)/bin:$PATH

@seankhliao

@seankhliao your assumption is correct. We do export PATH=$(pwd)/bin:$PATH in the project-level Makefile with the expectation that go generate ./... will find the various tools in that location.

The issue with injecting an absolute path into those //go:generate directives is that that absolute path is not the same on every machine.

As I said, it seems like updating those directives to use //go:generate ../../custom-tool ..... would technically work but that introduces the $PWD sensitivity I mentioned.

@dylan-bourque

@dylan-bourque, go:generate directions should not be sensitive to $PWD. Per the go:generate design doc, emphasis added:

command is the generator (such as yacc) to be run, corresponding to an executable file that can be run locally; it must either be in the shell path (gofmt) or fully qualified (/usr/you/bin/mytool) and is run in the package directory.

(And note that we currently use exactly that approach in the Go standard library: see https://golang.org/cl/261499.)

All of mine weren't sensitive to $PWD before. The commands being run were always located via the shell path, and we went out of our way to ensure that those would be the correct commands.

That is no longer the case with Go 1.15.7, though. The same //go:generate custom-tool .... commands that worked yesterday fail today.

and is run in the package directory

I see where my confusion was now.

As a test, I added //go:generate echo PWD is $PWD in a sub-package and ran go generate ./... from the project root. The output is the project root directory, not the sub-package. However //go:generate ls -l shows the contents of the sub-package folder.

Even with this new information, though, I still have hundreds of go:generate directives that were working perfectly before and now need to be checked and potentially updated to include a relative path to the command binary. ☹️

I think my point stands that not every invocation of a command within the current directory should be treated as malicious, though.

os/exec: add GODEBUG setting to opt out of ErrDot changes

os/exec: on Windows, suppress ErrDot if the implicit path matches the…

os/exec: add GODEBUG setting to opt out of ErrDot changes

Always prepend caddy command with dot (#117)

Is there a way to bypass this error at runtime in Go 1.18? I was under the impression that this was going to be released in Go 1.19 per the release notes but it looks like it also affects programs compiled with Go 1.18. We've got a few Go services deployed remotely at customer locations that can't self-upgrade due to this change, and we're looking for a way to bypass it temporarily.

@clarkmcc

@clarkmcc This change is not in Go 1.18. I suggest that you open a new issue with more details.

That said, if your Go 1.18 somehow has the Go 1.19 changes, perhaps it also has the documented GODEBUG change that disables it. See https://pkg.go.dev/os/exec.

This appears to cause substantial problems for go binaries that are run by Bazel and run subprocesses of their own because the directories in PATH are always relative to the cwd when running a command in Bazel. Bazel avoids using absolute paths by design in order to make builds more hermetic.

This appears to be blocking the use of gqlgen in Bazel because gqlgen depends upon golang.org/x/tools/go/packages, which uses golang.org/x/sys/execabs instead of os/exec, so I can't use the GODEBUG setting to opt out. :/

AFAICT, resolution of executables in nested directories doesn't seem like it's within the scope of the security vulnerability unless there's some other commonly-exploitable pattern of people putting relative paths on their PATH.

@Manbeardo

@Manbeardo Please open a new issue about these concerns. Thanks.

os/exec: return error when PATH lookup would use current directory

Navigation Menu

Repository navigation

os/exec: return error when PATH lookup would use current directory #43724

64 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

os/exec: return error when PATH lookup would use current directory #43724

Description

Activity

DmitriyMV commented on Jan 20, 2021

bcmills commented on Jan 20, 2021

mislav commented on Jan 20, 2021

dylan-bourque commented on Jan 22, 2021

seankhliao commented on Jan 22, 2021

dylan-bourque commented on Jan 22, 2021

bcmills commented on Jan 22, 2021

bcmills commented on Jan 22, 2021

dylan-bourque commented on Jan 22, 2021

dylan-bourque commented on Jan 22, 2021

64 remaining items

clarkmcc commented on Dec 13, 2022

ianlancetaylor commented on Dec 13, 2022

Manbeardo commented on Dec 13, 2022

Manbeardo commented on Dec 13, 2022

ianlancetaylor commented on Dec 13, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions