johnkerl / miller

Miller is like awk, sed, cut, join, and sort for name-indexed data such as CSV, TSV, and tabular JSON
https://miller.readthedocs.io
Other
9.03k stars 217 forks source link

[security] Shell command injection via filename when using --prepipe #111

Closed cym13 closed 8 years ago

cym13 commented 8 years ago

What

Using --prepipe to specify a preprocessing command is vulnerable to shell injection.

Proof of concept

[cym13@Eather test]$ ls
[cym13@Eather test]$ mlr --from a\;uname\ -a\;.csv --prepipe "echo" cat
sh: a: No such file or directory
1=Linux Eather 4.7.5-1-ARCH #1 SMP PREEMPT Sat Sep 24 13:04:22 CEST 2016 x86_64 GNU/Linux
sh: .csv: command not found

While it is well understood that prepipe allows the user to run shell commands, the fact that the filename isn't properly quoted is clearly a bug. All popen calls in miller are affected.

Hypothetical exploitation

Miller is a command-line tool which means it is likely to be integrated to scripts to deal with input automatically.

Let's consider the case of a website getting csv formatted input from users that wants them to be able to send a zipped file (think some accounting software linked to a website to analyze its data).

A malicious user could send a zip file with a specially crafted name like "a;python3 -m http.server;b.csv.zip" and send it, effectively executing remotely a python command launching a web server for further exploration.

This is a high-risk vulnerability that should be fixed ASAP.

johnkerl commented 8 years ago

Thank you for the detailed bug report!

johnkerl commented 8 years ago

58a801ee08984a547b214cc39b262c23c64693e6 single-quotes filenames to avoid the injection on read.

There is more to do in case of badness in the prepipe command itself, and there is more to do for escaping popen for output.

johnkerl commented 8 years ago
$ mlr --from 'a;uname -a;.csv' --prepipe "echo" cat
sh: a;uname -a;.csv: No such file or directory
johnkerl commented 8 years ago

This also needs to escape single-quotes in the "filename", if present. This still gets through:

mlr --from "a';uname -a;'.csv" --prepipe "echo" cat
johnkerl commented 8 years ago

As of b60e0c32be4d974661a049bf4f518160df4412ac that's blocked too:

$ mlr --from 'a;uname -a;.csv' --prepipe "echo" cat
sh: a;uname -a;.csv: No such file or directory

$ mlr --from "a';uname -a;'.csv" --prepipe "echo" cat
sh: a';uname -a;'.csv: No such file or directory
cym13 commented 8 years ago

That looks better. Don't torture yourself with “badness in the prepipe command”, as long as the user can control the command you'll never be able to stop him from doing bad things. Preventing bad things is the user's responsability here.

johnkerl commented 8 years ago

Thanks @cym13!