Command injection in Thor::Actions#get

[ecneladis]

Open-uri's open used in Thor::Actions#get allows executing system commands [1].

Documentation does not warn that source parameter is vulnerable to malicious input.

Example:

get "|uname -a > cmd_exec_test;\nhttps://gist.github.com", "/tmp/test_123"

[1] http://sakurity.com/blog/2015/02/28/openuri.html

[hoffoo]

can someone look into this? It is now published and failing in bundle-audit https://nvd.nist.gov/vuln/detail/CVE-2016-10545

Name: thor
Version: 0.20.0
Advisory: CVE-2016-10545
Criticality: Unknown
URL: https://github.com/erikhuda/thor/issues/514
Title: Command injection in Thor Gem
Solution: remove or disable this gem until a patch is available!

Vulnerabilities found!```

[pherris]

To ignore this warning until a patch is available: bundle-audit check --ignore CVE-2016-10545

[xymbol]

Is someone working on this? How can we help? /cc @rafaelfranca

[rafaelfranca]

O_0. Who asked for that CVE without discussion this with the project maintainers?

[xymbol]

@rafaelfranca I don't know, IIRC it was recorded as reported by MITRE.

Just reviewed the code and the specs. I understand the vulnerability around open but don't see how a public exploit would work in this case. I don't recall user input in well-behaved apps ever being passed onto Thor actions.

@galori It's not trivial to disable if, say, you're using bundler-audit's task.

Maybe the original submitter can chime in? /cc @ecneladis

[rafaelfranca]

@reedloden do you know who submitted that CVE? MITRE says it came from HackerOne

[xymbol]

According to specs, seamless file and url access are by design. In this case, getting open out of the way would not reduce any risk.

[rafaelfranca]

Yes, this is by design. Thor is a system tool that unlikely will receive user input. Saying thor is vulnerable for command injection is the same thing than saying bash is vulnerable for command injection.

[ghiculescu]

rubysec/ruby-advisory-db#341 - I've proposed that ruby-advisory-db remove this as the consensus here seems to be that this isn't going to get changed in Thor.