iKostanOrg/codewars

Starting a process with a shell, possible injection detected, security issue.

Closed this issue · 2 comments

ikostan commented

Codacy detected an issue:

Message: Starting a process with a shell, possible injection detected, security issue.

Occurred on:

Currently on:

ikostan commented

Some useful info here

Rather than passing a string to subprocess, our function passes a list of strings. The ping program gets each argument separately (even if the argument has a space in it), so the shell does not process other commands that are provided by the user after the ping command terminates. You do not have to explicitly set shell=False - it is the default.

ikostan commented

Even more relevant topic here

It has security issues just when you run the function with arguments taken from users. For example:

import os
def do_clear(command): # Notice command is sent as argument from outside world and hence this makes it vulnerable
    os.system(command)

If the method is called with for example:

do_clear('rm -f */*')

Then it is possible that it deletes all the files of current directory. But if the 'clear' command is to be directly used, you do not have to worry about the security issue, as only 'clear' is run in all conditions. So the following function is secure enough.

def do_clear(): # Notice command is not sent as argument from outside world
    os.system('cls' if os.name == 'nt' else 'clear') # This is not risky as os.system takes clear/cls command always.