Search code examples
python-3.xbashshellinputsanitization

Python3 - Sanitizing user input for shell use


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?


Solution

  • 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:

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