Github proposal: os/exec: make LookPath not look in dot implicitly on Windows ·...

What version of Go are you using (go version)?

$ go version
go version go1.14.1 windows/amd64

Originally noticed with Go 1.8 though.

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\holmed2\AppData\Local\go-build
set GOENV=C:\Users\holmed2\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\holmed2\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\holmed2\AppData\Local\Temp\go-build226326482=/tmp/go-build -gno-record-gcc-switches

What did you do?

Copy “C:\Windows\System32\whoami.exe” to the following program’s directory as “systeminfo.exe” and then run the program:

package main

import (
      "fmt"
      "os"
      "os/exec"
)

func main() {
      os.Setenv("PATH", `C:\Windows\System32`)

      cmd := exec.Command("systeminfo.exe")
      out, err := cmd.CombinedOutput()
      if err != nil {
            panic(err)
      }
      fmt.Println(string(out))
}

What did you expect to see?

The output of C:\Windows\System32\systeminfo.exe

What did you see instead?

The output of ./systeminfo.exe (my username, since it is a copy of whoami.exe)

If the renamed copy of systeminfo.exe is removed from the test program’s directory, then the output of “C:\Windows\System32\systeminfo.exe” is displayed as expected.

Analysis

os/exec/lp_windows.go contains the following:

func LookPath(file string) (string, error) {
…
      if strings.ContainsAny(file, `:\/`) {
            if f, err := findExecutable(file, exts); err == nil {
                  return f, nil
            } else {
                  return "", &Error{file, err}
            }
      }
      if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
            return f, nil
      }

If the value of ‘file’ is an absolute or relative path, a result is returned. The concern is with a value which is only a name, such as “systeminfo.exe”. One would expect this to search the list of paths found in the PATH environment variable, but before doing so the code explicitly searches the current working directory (“.”). There does not appear to be any means provided to disable this behavior and search only PATH.

I would guess that the intent was to mimic the behavior of the cmd.exe command shell, which searches the current directory first even if it is not specified in PATH. By comparison, the documentation for the Windows CreateProcess API indicates that it does not search PATH at all, but will use the current directory to complete a partial path. (The SearchPath API offers an alternative, though also flawed, option to search the current directory last.)

The problem is that it is not possible to use exec.LookPath, and thus exec.Command, to search the system PATH without searching the current directory. Thus even if diligence is taken to have the program set a secure PATH value, the programmer must be aware of this behavior and avoid using these standard library functions. The documentation of exec.LookPath does not mention the current directory, stating only that it searches “the directories named by the PATH environment variable.”

Suggestions

My preferred recommendation would be to remove the explicit search of “.” (the second if-clause shown above), in order to provide the best level of security and comply with the documentation. A programmer can add “.” to the PATH environment variable value if the behavior is desired, as one would do in a linux/unix program.

If the resulting change in Go behavior/compatibility is not desirable, a workaround could be for exec.LookPath to reference the NoDefaultCurrentDirectoryInExePath environment variable and avoid searching “.” if it is set. This is a workaround which Microsoft apparently added in Vista to disable the behavior in cmd.exe.

cc @FiloSottile