Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subprocess input data warning on literal command #373

Open
GoldsteinE opened this issue Aug 17, 2018 · 5 comments
Open

Subprocess input data warning on literal command #373

GoldsteinE opened this issue Aug 17, 2018 · 5 comments
Labels
bug Something isn't working
Milestone

Comments

@GoldsteinE
Copy link

Describe the bug
Bandit reports when user is trying to call subprocess, even when command is static.

To Reproduce
Create test.py:

import subprocess

def f():
    print(subprocess.check_output(['/usr/bin/ls']))

Call:

$ bandit test.py

Expected behavior
Bandit shouldn't report B603 on line 4, because there is no way untrusted input appear there.

Bandit version

bandit 1.5.0
  python version = 3.5.2 (default, Nov 23 2017, 16:37:01) [GCC 5.4.0 20160609]
@tleeuwenburg
Copy link

tleeuwenburg commented Aug 27, 2018

I was taking a look at this. I can't actually think of a safe way to reliably ascertain this at runtime. It is possible to check the argument using bandit, but I can't work out how to distinguish the situation where a string is formatted using a variable one line above and then supplied in. The original source code could maybe be loaded through introspection at runtime and checked but I'm not confident this approach is rock-solid.

Here are a bunch of example signatures and whether I think they are safe or not:
# This is safe because the command is immutable and a new shell is not used
SAFE_STATIC_SHELL = '''
import subprocess

def f():
    print(subprocess.check_output(['/usr/bin/ls']))
'''

# This is unsafe even though the comment is immutable because environment
# variables could be used to modify execution or paths
UNSAFE_STATIC_SHELL = '''
import subprocess

def f():
    print(subprocess.check_output(['/usr/bin/ls'], shell=True))
'''

# This is unsafe because a variable is used to create the command that
# is being run and could therefore be user-modifiable
UNSAFE_STATIC_SHELL2 = '''
import subprocess

def f(command = /usr/bin/ls'):
    print(subprocess.check_output(['%s' % command], shell=False))
'''

# This is unsafe because a variable is used to create the command that
# is being run and could therefore be user-modifiable
UNSAFE_STATIC_SHELL3 = '''
import subprocess

def f(command = /usr/bin/ls'):
    print(subprocess.check_output([command], shell=False))
'''

@Tatsinnit
Copy link

Tatsinnit commented Nov 9, 2018

+1 and it will be awesome if we can get some official take on this one please (i.e. literal command command in subprocess.checkout throws warning), thanks.

@raratiru
Copy link

Bandit reports this even when performing a self test:

bandit -iii -r  bandit

...

>> Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input.
   Severity: Low   Confidence: High
   Location: bandit/cli/baseline.py:107
   More Info: https://bandit.readthedocs.io/en/latest/plugins/b603_subprocess_without_shell_equals_true.html
106	            try:
107	                output = subprocess.check_output(bandit_command)
108	            except subprocess.CalledProcessError as e:

...

@ericwb ericwb added this to the Near Future milestone May 9, 2019
@ericwb ericwb added the bug Something isn't working label May 9, 2019
@skwashd
Copy link

skwashd commented Nov 12, 2019

@ericwb can you provide some guidance on how this issue should be resolved? This is blocking our rollout of bandit on a couple of projects. I have a developer who can work on the fix, but it would help if you can point them in the right direction.

@amacfie
Copy link
Contributor

amacfie commented Jan 1, 2020

Duplicate of #333

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants