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?

CodePudding user response：

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

user_input="file1.txt file2.txt"
subprocess.Popen(['cp']   user_input.split())

If you want to increase security further, you could:

• explicitly set 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)
  

• use absolute paths for command (to prevent injection of malicious executables via PATH):

  subprocess.Popen(['/usr/bin/command', user_input])
  

• explicitly instruct commands that support it to stop parsing options, e.g.

  subprocess.Popen(['rm', '--', user_input1, user_input2])
  

• do as much as you can natively, e.g. 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)