Command Injection Vulnerability CWE-77

[Demuxx]

The grpc call to ps doesn't filter input, allowing injection of arbitrary bash commands. There should be a check of the req.Args to filter out special characters, probably whitelisting [,a-zA-Z]. Credit to @iammyr for finding this.

https://github.com/kata-containers/agent/blob/eec68398287d9491fe648a8e54fb942cf6b6d934/grpc.go#L832

[grahamwhaley]

Thanks for the advisory @Demuxx (and by implication @iammyr). Any hints on how this was found - by code reading or automated scanning perhaps? Do you have any concrete examples of using this gap? Can it be deployed from the docker command line for instance, or would somebody need to gain access to the grpc endpoint?

I'm wondering as that might:

• guide us to a fix
• help us consider other cases, like I would presume maybe abuse of docker exec, depending on the attack mode.

/cc @devimc I think as the implementer of ps ?

[devimc]

Can it be deployed from the docker command line for instance ..

I don't think so, since we don't have support for docker top

[devimc]

sorry I can't reproduce it, even using exec.Command directly

cmd := exec.Command("ps", ";ls")
cmd := exec.Command("ps", "||ls")
cmd := exec.Command("ps", "^ls")
cmd := exec.Command("ps", "&ls")
cmd := exec.Command("ps", ";", "ls")
cmd := exec.Command("ps", "||", "ls")
cmd := exec.Command("ps", "&&ls")
cmd := exec.Command("ps", "$(ls)")
cmd := exec.Command("ps", "`ls`")
.....
.....

[jodh-intel]

Hi @devimc - do you mean "can't reproduce"? :)

[devimc]

@jodh-intel hehe yes, updating comment, thanks

[Demuxx]

This is probably me jumping the gun on vulnerability findings again. The tool used was gosec. I'm guessing this is being filtered by Go exec.Command?

[Demuxx]

Yep, false positive. Very sorry for the false alarm!

[jodh-intel]

No problem - it's great that you're analysing the code! :)

... which makes me wonder...

We run gometalinter as part of our CI: see

• https://github.com/kata-containers/tests/blob/master/.ci/static-checks.sh#L202..L264

It would be great if you and/or @iammyr would be willing to whip up a PR to add support for running gosec (which is supported by gometalinter) :smile: