I am busy writing a Python3 script which requires user input, the input is used as parameters in commands passed to the shell.
The script is only intended to be used by trusted internal users - however I'd rather have some contingencies in place to ensure the valid execution of commands.
Example 1:
import subprocess
user_input = '/tmp/file.txt'
subprocess.Popen(['cat', user_input])
This will output the contents of '/tmp/file.txt'
Example 2:
import subprocess
user_input = '/tmp/file.txt && rm -rf /'
subprocess.Popen(['cat', user_input])
Results in (as expected):
cat: /tmp/file.txt && rm -rf /: No such file or directory
Is this an acceptable method of sanitizing input? Is there anything else, per best practice, I should be doing in addition to this?
The approach you have chosen,
import subprocess
user_input = 'string'
subprocess.Popen(['command', user_input])
is quite good as command
is static and user_input
is passed as one single argument to command
. As long as you don't do something really stupid like
subprocess.Popen(['bash', '-c', user_input])
you should be on the safe side.
For commands that require multiple arguments, I'd recommend that you request multiple inputs from the user, e.g. do this
user_input1='file1.txt'
user_input2='file2.txt'
subprocess.Popen(['cp', user_input1, user_input2])
instead of this
user_input="file1.txt file2.txt"
subprocess.Popen(['cp'] + user_input.split())
If you want to increase security further, you could:
shell=False
(to ensure you never run shell commands; this is already the current default, but defaults may change over time):
subprocess.Popen(['command', user_input], shell=False)
command
(to prevent injection of malicious executables via PATH
):
subprocess.Popen(['/usr/bin/command', user_input])
subprocess.Popen(['rm', '--', user_input1, user_input2])
cat /tmp/file.txt
could be accomplished with a few lines of Python code instead (which would also increase portability if that should be a factor)