10

Ok, I have a some command wich MUST be executed in shell=True mode.

os.system or subprocess.Popen(..., shell=True)

And this command contain string substitution like: cmd = "some_secret_command {0}".format(string_from_user)

I want escape string_from_user variable to prevent ANY injections.

Simple wrong answers:

  1. Use shlex.quote - incorrect

print(shlex.quote('file.txxt; &ls . #')) -> 'file.txxt; &ls . #' (injection)

Example:

> python -c "import sys; print(sys.argv[1])" 'file.txxt; &ls . #'
secret.txt
secret2.txt
  1. Use escape ^ - incorrect

Example:

import os

CMD = '''string with spaces'''.replace('', '^').replace('^"', '')
os.system('python -c "import sys; print(sys.argv[1])" {0}'.format(CMD))

Now I can use (space) and inject more then one argument.

  1. Use ^ and " or ' - incorrect

Example:

import os

CMD = '''some arg with spaces'''.replace('', '^').replace('^"', '')
os.system('python -c "import sys; print(sys.argv[1])" "{0}"'.format(CMD))

print ^s^o^m^e^ ^a^r^g^ ^w^i^t^h^ ^s^p^a^c^e^s^

and if '

import os

CMD = '''some spaces'''.replace('', '^').replace('^\'', '')
os.system('python -c "import sys; print(sys.argv[1])" \'{0}\''.format(CMD))

print 'some

I now about shell=False but this is incorrect for me.

Holger Just
  • 52,918
  • 14
  • 115
  • 123
pahaz
  • 837
  • 9
  • 13
  • 1
    You have the Windows tag, can you confirm which shell you intend to run, e.g. cmd.exe? The reason I ask is because the quoting required differs, Powershell uses back-tick, for example. – cdarke Mar 23 '15 at 14:46
  • 1
    My mistake, I mean `cmd.exe`. – pahaz Mar 23 '15 at 14:50
  • 1
    Why do you need a shell? I don't know why you would **need** one. – Dunes Mar 23 '15 at 16:54

2 Answers2

18

The problem with quoting command lines for windows is that there are two layered parsing engines affected by your quotes. At first, there is the Shell (e.g. cmd.exe) which interprets some special characters. Then, there is the called program parsing the command line. This often happens with the CommandLineToArgvW function provided by Windows, but not always.

That said, for the general case, e.g. using cmd.exe with a program parsing its command line with CommandLineToArgvW, you can use the techniques described by Daniel Colascione in Everyone quotes command line arguments the wrong way. I have originally tried to adapt this to Ruby and now try to translate this to python here.

import re

def escape_argument(arg):
    # Escape the argument for the cmd.exe shell.
    # See https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way
    #
    # First we escape the quote chars to produce a argument suitable for
    # CommandLineToArgvW. We don't need to do this for simple arguments.
  
    if not arg or re.search(r'(["\s])', arg):
        arg = '"' + arg.replace('"', r'\"') + '"'
  
    return escape_for_cmd_exe(arg)

def escape_for_cmd_exe(arg):
    # Escape an argument string to be suitable to be passed to
    # cmd.exe on Windows
    #
    # This method takes an argument that is expected to already be properly
    # escaped for the receiving program to be parsed by it. This argument
    # will be further escaped to pass the interpolation performed by cmd.exe
    # unchanged.
    #
    # Any meta-characters will be escaped, removing the ability to e.g. use
    # redirects or variables.
    #
    # @param arg [String] a single command line argument to escape for cmd.exe
    # @return [String] an escaped string suitable to be passed as a program
    #   argument to cmd.exe

    meta_re = re.compile(r'([()%!^"<>&|])')
    return meta_re.sub('^\1', arg)

Applying this code, you should be able to successfully escape your parameters for the cmd.exe shell.

print escape_argument('''some arg with spaces''')
# ^"some arg with spaces^"

Note that the method is expected to quote a single complete argument. If you are collecting your argument from multiple sources, e.g., by building a string of python code to pass to the python command, you have to assemble this before passing it to escape_argument.

import os

CMD = '''string with spaces and &weird^ charcters!'''
os.system('python -c "import sys; print(sys.argv[1])" {0}'.format(escape_argument(CMD)))
# string with spaces and &weird^ charcters!
Holger Just
  • 52,918
  • 14
  • 115
  • 123
  • This answer is great and works when you start a Python subprocess, unfortunately I am starting a .bat file as subprocess and as soon as I do `echo %1%` everyhing gets mangled again :/ – xjcl Jan 27 '22 at 18:17
  • @xjcl Then you should show your concrete situation with a minimal example. Using `echo %1` isn't the best idea with complex arguments – jeb May 09 '22 at 16:46
  • I'm just using subprocess to start a `.bat` file that literally only contains `echo %1%` – xjcl May 09 '22 at 18:17
  • @xjcl Your batch file must to be updated to deal with quoted arguments. See e.g. https://stackoverflow.com/a/4388728/421705 – Holger Just May 09 '22 at 19:19
  • I don't think this is the right way to go for Windows, either. For example, the ';' character is not even looked at for command-line arguments. If someone enters a ; into a string with cmds then it will end the previous command and allow them to start injecting a new command. This is a very obvious thing to look for and filter but it wasn't covered making me not want to trust this. – Matthew Roberts Jun 25 '22 at 00:18
1

shorter (no other improvement on accepted):

import re

cmd_meta_re = re.compile(r'([()%!^"<>&|])')
cmd_needs_quote = re.compile(r'["\s]')
def cmdquote(s):
    s = cmd_meta_re.sub(r'^\1', s)
    if not s or cmd_needs_quote.search(s):
        s = '"%s"' % s.replace('"', '\\"')
    return s
Jonathan Graehl
  • 9,182
  • 36
  • 40
  • The replacement for quotes `"` -> `\"` is a bad idea, because for batch files only the caret `^` is an escape character, but not the backslash `\`. Escaping the parentheses and the exclamation mark is useless. The percent sign can't be escaped on the command line at all, but carets still help in some situations – jeb May 09 '22 at 16:43
  • I think your order is wrong. You need to first add quotes to the argument if required (for the CommandLineToArgvW parsing by the actual program), and then escape those quotes (to circumvent the parsing rules of cmd.exe). To quote the article linked to in my answer: "It's important to also `^`-escape `"` characters: otherwise, cmd would literally copy `^` characters appearing between `"` pairs, mangling the argument string in the process. [...] In effect, because cmd's `"` handling is useless for our purposes, we use `^` to tell cmd to not attempt to be smart about quote detection." – Holger Just May 09 '22 at 23:46