-
Notifications
You must be signed in to change notification settings - Fork 18k
os/exec: execution of batch-files (.cmd/.bat) is vulnerable in go-lang for windows / insufficient escape #27199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
os/exec
: execution of batch-files (.cmd/.bat) is vulnerable in go-lang for windows / insufficient escape
What is interpreting the special characters? It's not immediately clear to me that this should be the responsibility of the os/exec package. But I don't know much about things work on Windows. |
What do you mean exactly? In case of execution of the process (no matter exe-file of bat- or cmd-file) with arguments (so as safe method with argument-list, not using shell-syntax command line) - the special meta-chars should not cause any behavior, allowing to produce an injection, that can cause execution of something else as the process self.
So whose responsibility is should be in your opinion? |
BTW as regards the "Go1.12 milestone"... |
Could you point to relevant msdocs? Generally users should be aware that if they execute a file windows will look up the
There's this: https://msdn.microsoft.com/en-us/library/a1y7w461.aspx but it seems that cmd.exe has it's own argument parse algorithm indeed. |
FWIW (suspected) security vulnerabilities should be disclosed privately (see https://golang.org/security). |
http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
Generally any program can do with supplied arguments what it wants. But this is not a reason to disregard the common rules how arguments will be unescaped back inside the windows process.
Yes. It is still worse in windows, because the whole command-line will be build together, before the child-process invoked, and takes splitting (parsed back) into argv by the CRT-library or CommandLineToArgv, etc inside the child-process (whatever it is).
You don't really want to compare the execution from go-lang with execution within command-processor (inside the batch-file)? Because this way the args you supplied as command will be processed inside the COMSPEC (still BEFORE the execution occurs there).
It does no matter. So again, I told about common escape rules (exe-file, batch-file) and not how one can define it's own handling in the child-process. |
I mean https://msdn.microsoft.com/en-us/library/17w5ykft.aspx doesn't mention that "test&whoami" requires special escaping. (Btw: I'm not part of the go project at all). I was just curious if there's documentation available for how this process exactly works because there must be something inherently different in how the microsoft crt parses arguments than to how they are parsed for the command interpreter so that one could fix this manually in vulnerable projects/code/dependencies one uses. You propose an algorithm as a solution but how can one verify that your solution is actually correct and covers everything without having documentation to verify this? |
So could you call me the person, which I could reach "privately"? :) But I'm agree, if it would be absolutely 100% reproducible vulnerability, where for example some service is affected by supplying of some concrete arguments - that is the right way. Savvy? |
There is no documentation as one place (as well as no procedure in windows like And one really wonders that MS has no (or rather few) docu for something? This is normal case unfortunatelly for people developing for this platform.
As I wrote "my algo makes it correct", it was just the answer to "assoc and ftype" question, so I meant just the algorithm works proper for both cases equally (exe and batch). If someone meant that the writing an expensive PoC, several test cases and analysis, creating an issue with description is still not enough, he can try to do it better...
For example if one write more test-cases resp. extend they to reach better coverage. |
I looked into this and it does appear to be a real issue. Background On Windows, command line arguments are not passed to programs as an array like they are on UNIX, but as a single, unprocessesed string. In principle, this means that any program is free to parse the command line string any way it wants, using any quoting rules it wants. In practice, most programs use the CommandLineToArgv function to parse command line arguments, so Go's os/exec package quotes the argument array in such a way that CommandLineToArgv will yield the same arguments. The problem Most windows programs use CommandLineToArgv to parse their arguments, but there is an important exception: cmd.exe. Also, .BAT and .CMD files are executed on windows by running CMD.EXE /C. If you run a .BAT file with arguments, as in e.g.,
then windows ends up running
The part after All together, this means that if a Go program ever executes a .bat file with os/exec, it is likely that the command line string will be misinterpreted. Since CMD.EXE can execute arbitrary commands, this leads directly to arbitrary code execution. @sebres does that about sum it up? Incidentally, it looks like this problem was already recognized and reported two years ago (#15566, maybe #17149) and a fix proposed (https://golang.org/cl/30947) but never merged. |
Minimal reproducer: exec.go package main
import (
"log"
"os"
"os/exec"
)
func main() {
cmd := exec.Command(`echo.bat`, "&whoami")
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Run()
if err != nil {
log.Fatal(err)
}
} echo.bat
output
expected output
|
This already been discussed in other issues. There is no point repeating all that here too. Alex |
Thus closed as "unlikely to be implemented" resp. as "duplicate". There are two categories of people: one of that says "the world is unfair" (MS must die, etc), and another that try to repair this "broken" world. |
Change https://golang.org/cl/132695 mentions this issue: |
I am happy to repair this "broken" world, but I do not have free time to spend on this issue. If you have proposal to fix cmd.exe command line building, this https://golang.org/doc/contribute.html is how to contribute your solution. I will help you in whatever way I can. Alex |
Updates #27199 Change-Id: I5cb6540266901697d3558ce75b8de63b1bfc2ce0 Reviewed-on: https://go-review.googlesource.com/132695 Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
We established that
Go is a platform independent language. Why should it be aware of a platform specific plumbing rule in Windows that launches a broken argument parser? This point was already brought up, however: How often does the adversary control a command line argument but not the executable name (which would have to be a batch file)? In what case would executing such ambiguous arguments result in a change in scope that gets the adversary more access than they already have on the system? The linked issue page ranks several languages and uses a subjective scoring system to emphasize priority. But there are established scoring systems for vulnerabilities (e.g., https://www.first.org/cvss/calculator/3.0) that are more useful. The priority of this issue appears inflated with respect to its actual surface area. Are there any data that demonstrate examples of this issue in practical Go programs? |
Execution of batch-files using
os/exec
with arguments containing some special meta-characters is vulnerable and may be used to execute foreign data/code.What version of Go are you using (
go version
)?Latest stable build: go1.10.3 windows/386
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?Windows / x86
What did you do?
Execution of batch-file using
os/exec
with arguments containing some special meta-characters.A recipe for reproducing the error as well as more extensive PoC with additional info (and more lang's affected also) - github/sebres/PoC/SB-0D-001-win-exec
A complete runnable program - test-dump-part.go:
Content of test-dump-part.go
An example:
For more "broken" cases see the result of my test-suite:
https://github.com/sebres/PoC/blob/master/SB-0D-001-win-exec/results/go.diff
What did you expect to see?
Arguments are escaped/quoted properly.
What did you see instead?
Arguments are insufficient escaped/quoted, so it is vulnerable currently.
Solution:
For possible solution see the algorithm description resp. how it was fixed in TCL (see the function
BuildCommandLine
)Possible similar issues:
#17149, #3752
The text was updated successfully, but these errors were encountered: