Repository navigation
proposal: os/exec: make LookPath not look in dot implicitly on Windows #38736
Description
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?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ 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
Activity
as commentedon Apr 29, 2020
Disabling this will likely break dockerized windows containers relying on the behavior. Amusingly, creating
ping.batwill makecmd.exerun it before the system's ping command, as windows has an extension search list too.But, if you can copy a file in the same directory as the executable, you can probably rename the original executable (even while its process is running) and replace it with a program that deletes all files on your filesystem next time its executed. The security community treats the filesystem as the wild wild west, but the only way to mitigate hijacking attacks is to properly secure the filesystem in the first place.
Hence, I think Go should not try to solve this problem if LookPath is designed to be compatible with Windows behavior (but NoDefaultCurrentDirectoryInExePath might be a good idea, depending on how it works with modern Windows versions).
[-]Unsecure path search in os/exec[/-][+]os/exec: LookPath implicitly searches current directory[/+]aral commentedon Jul 23, 2020
I’d consider this desirable behaviour, if consistent cross-platform.
e.g., Use case:
mkcert uses the following call to find certutil if it is installed:
I’m going to be bundling pre-built binaries of certutil with future version of Auto Encrypt Localhost. Afaics, with the current behaviour, all I have to do is place the certutil binary and the nss dynamic libraries in the same folder as the mkcert binary. If I had to specify the folder explicitly, I’d most likely have to maintain a fork of mkcert.
mislav commentedon Oct 27, 2020
@dholmesdell You are right that this was the motivation for the feature: 2a876be
However, I would argue that this was a bad call, as it was made 9 years ago with little discussion about pros & cons. I don't see why the shell feature of
cmd.exeto execute binaries or batch scripts from the local directory was worth propagating in Go, especially since that feature wasn't kept in PowerShell, which is now de-facto standard shell in Windows.This behavior of LookPath on Windows absolutely goes against the documentation of the LookPath method, and I find it surprising at best and a security vulnerability at worst.
/cc @alexbrainman @rsc for comments
rsc commentedon Oct 28, 2020
The behavior we chose matched the behavior of the default Windows shell at the time.
It seems unlikely we could change it now without breaking many Windows programs.
mislav commentedon Oct 29, 2020
@rsc I understand; thank you for chiming in.
Would you be open to a documentation change that points out this behavior on Windows?
dawidgolunski commentedon Oct 29, 2020
Hi. As for executing new processes, I thought the Windows standard order (other than cmd that mimics DOS) of searching directories was the one used by the CreateProcessA in win32 API which only searches the current directory as the second option, only if the command was not found in the directory from which the application loaded . See:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessa
I think this is what python's subprocess.Pipe() method mimics on Windows for example.
I personally saw it as a vuln and even requested a CVE for this issue but it seems like the agreement here is that it can't be changed in Go itself and programmers must take care of this individually.
What I think would be helpful in this case, in addition to clarifying the documentation, is adding a separate function to GO that only searches the directories declared in PATH without including cwd/. E.g. StrictPath() in the exec package?
This way existing apps won't be broken and programmers can use this function without re-inventing the wheel when needed.
Just an idea. Would this work ? Could this be added?
ianlancetaylor commentedon Oct 29, 2020
@mislav Documentation changes are fine.
@dawidgolunski As far as I can tell
exec.LookPathon Windows does not search the directory from which the application loaded at all. It searches the current directory, then the directories on the environment variablepath.I could imagine a
exec.LookPathStrictproposal. Do you want to write one (https://golang.org/s/proposal)? It could perhaps ignore.or any relative directory on thePATH/path.dawidgolunski commentedon Oct 29, 2020
@ianlancetaylor That's right,
LookPathfunction does not do that. I was referring to theCreateProcessAfunction (processthreadsapi.h) from the Windows API. The Microsoft article I sent in the previous message defines the executable file search order (if the full path was not provided) as follows:1. The directory from which the application loaded.
2. The current directory for the parent process.
...
6. The directories that are listed in the PATH environment variable.
The step 1 adds a layer of protection as usually installed applications are loaded from directories owned by admin/SYSTEM user which prevents modification as regular users can't add files into them.
Good name I think. As for the proposal, I'll try to have a go at it when I get a spare minute.
31 remaining items
tarenshit commentedon Feb 6, 2021
hugo: Update to 0.80.0
hugo: Update to 0.80.0
KikeE36 commentedon Feb 27, 2022
9315398773 commentedon Jun 10, 2022
9315398773 commentedon Jun 10, 2022
rafasha123 commentedon Jun 19, 2022
xurKiz-cykfip-covki8 commentedon Jul 8, 2022
bcmills commentedon Jul 8, 2022
Proposal #43947 was accepted and implemented for this instead.