Comment 5 for bug 1450798

Revision history for this message
Travis McPeak (travis-mcpeak) wrote :

Michael - the code you mention in processutils is a wrapper around Popen, and one of its parameters is "shell". Setting shell to True will cause the command to run in a shell with shell capabilities, including the ability to "escape" one command and start another with the semicolon character.

When the following is provided as input to to a shell=True call, it executes two commands (ls and rm)

"ls /my/dir; rm /etc/passwd". The issue is that if an input is constructed taking the second part of this "/my/dir; rm /etc/passwd" it will execute a new command. The safe way to use this is to call without shell and parameterize the call. So you call:

POpen(mycommand, my_argument, my_argument2) without running on a shell.

If we're 100% sure that none of the inputs come from an untrusted or less trusted user, then I agree with you - there is no risk. However I'd still claim that we should fix this as:

1) it isn't best secure development practice
2) you never know when the assumption of clean user input might change and a new threat vector is created
3) shelling this command doesn't appear to be required