praetorian-inc / gokart

A static analysis tool for securing Go code
Apache License 2.0
2.18k stars 110 forks source link

detect when external command running is vulnerable to option injection #15

Open pabs3 opened 3 years ago

pabs3 commented 3 years ago

I noticed that gokart supports checking for command injection. Another related thing is when injecting command-line parameters allows you to inject parameters that get interpreted as options. Some options change the behaviour of commands in suboptimal ways, but some options to some commands can result in arbitrary code execution. The canonical paper about this type of attack is Back To The Future: Unix Wildcards Gone Wild by Leon Juranic of DefenseCode. It would be great if gokart could support detecting option injection.

isp1r0 commented 3 years ago

@pabs3 Many thanks for the suggestion and link! We're definitely interested adding more capabilities to our analyzers, especially ones that are suggested by the community. We started diving into some examples from https://gtfobins.github.io/#+shell and realized this would be better sourced from within the community.

Our intuition is that Option Injection will be prevalent when devs have a need to shell out and most likely occur where Go standard libraries have limited functionality as compared to standard cmd utilities, such as using find() to search within directories rather than using the path module.

Do you have any suggestions for these cases in Go where this anti-pattern is commonly found? Code examples would favored but any pointers, hints or links appreciated as well :)

pabs3 commented 3 years ago

I don't recall seeing this issue in any Go code yet, but I definitely recall seeing it in other languages, but I don't have any CVEs at hand. Indeed, this only occurs when executing programs external to the current process. Your intuition about where it occurs sounds right. There probably is code to be found on GitHub or Debian CodeSearch if you issue a query for Go code that uses process execution functions.

https://codesearch.debian.net/search?q=filetype:go+exec&literal=1 https://github.com/search?l=Go&q=exec&type=Code

I did some searching and found a vulnerable function here, but it looks like the rest of the code only passes it temporary dirs:

https://sources.debian.org/src/golang-github-tonistiigi-fsutil/0.0%7Egit20200331.f427cf1-2/bench/bench_test.go/?hl=133#L132

So this feature will definitely require some sort of taint analysis, otherwise it is going to produce many false positives.

BTW, there are even more complicated versions of this, such as when running git clone, passing arbitrary parameters with unsafe protocol names results in code execution (CVE-2018-7032 in myrepos was that).

https://bugs.debian.org/840014

-- bye, pabs

https://bonedaddy.net/pabs3/

isp1r0 commented 3 years ago

Hey @pabs3 - thanks for the details here! It's a super interesting thread and love that you've continued to pursue this capability.

After some further investigation, I believe we can add a check for this once we've gotten a more flexible (and accurate) way of identifying Sinks with respect to the parameters being passed in.

Currently we're identifying all function parameters to a given vulnerable function as tainted when we really want to perform specific checks, i.e. second parameter to a exec.Command should only be treated as tainted if the first has a parameter with something like "-sh". In a similar fashion, we could look for an "rsync" or other option-injectable command and IFF we see that as the first parameter would be look for user-controllable data going to a subsequent parameter...

BTW: we released a new version of GOKart last Friday with remote repo scanning and some output options. I've found it pretty sweet for scanning repos in a nice frictionless fashion. Check it out if you get a chance!