Skip to content

Conversation

enricobacis
Copy link

The use of shell=True in subprocess is highly discouraged and can have unexpected (in the best of the scenarios) or even disastrous consequences.

When using shell=True in fact, the string is directly passed to the shell, so what happens when you use operators such as: ;, !, >, >>, <, <<, &, &&, |, || ?

These are directly processed by the shell, and since these are common symbols in a programming language it may lead to really bad consequences. For example try to use this:

open a file in php | wc -l

The output is 11, because there are 11 lines in the howdoi output.

use the operator > in c

This will create a file in since > redirects the output of howdoi use the operator to the fine in.

This can lead to really bad consequences. I hope nobody ever looked for how to use | to rm -rf..

subprocess.Popen takes a list of strings as input (the argv), so you just need to remove shell=True and use ['howdoi', query] as argument and you are on the safe side!

The use of `shell=True` in subprocess is highly discouraged and
can have unexpected (in the best of the scenarios) or even
disastrous consequences.

When using `shell=True` in fact, the string is directly passed
to the shell, so what happens when you use operators such as:
`;`, `!`, `>`, `>>`, `<`, `<<`, `&`, `&&`, `|`, `||` ?

These are directly processed by the shell, and since these are common
symbols in a programming language it may lead to really bad
consequences. For example try to use this:

    open a file in php | wc -l

The output is `11`, because there are 11 lines in the `howdoi` output.

    use the operator > in c

This will create a file `in` since `>` redirects the output of `howdoi
use the operator` to the fine `in`.

This can lead to really bad consequences. I hope nobody ever looked for
how to use `|` to `rm -rf`..

`subprocess.Popen` takes a list of strings as input (the `argv`), so
you just need to remove `shell=True` and use `['howdoi', query]` as
argument and you are on the safe side!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant