7

Github proposal: os/exec: return error when PATH lookup would use current direct...

 3 years ago
source link: https://github.com/golang/go/issues/43724
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Contributor

rsc commented 15 days ago

edited

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:

  • #38736, which removes the implicit dot lookup from LookPath on Windows
  • #42420, to add LookPathAbs that doesn't return relative results.
  • #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"
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.


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK