Skip to content

Do not apply sandbox-process-creation when arguments are literals #263

Closed

Description

@drdavella

Description

We may encounter cases where all of the command arguments to a subprocess call are hard-coded literals. In these cases hardening does not really make any sense and should be skipped.

A simple implementation would be to detect whether the immediate argument is a literal list that contains only literal strings. However a more sophisticated implementation would involve some constant propagation. As part of this issue we should explore the level of effort involved in the latter approach. If it's too difficult, implement the simpler approach for now and open another ticket.

This issue was initially described in pixee/python-security#10

Activity

  1. clavedeluna commented on Feb 18, 2024

    @clavedeluna
    Contributor

    The test cases here would then be:

    don't sandbox these:

    # str literal and user should know what they wrote
    subprocess.run("ls & cat /etc/passwd", shell=True)
    
    # don't sandbox, it's a list of str literals
    subprocess.call(["ls", ";cat", "/etc/passwd"], shell=True) 
    
    # hard-coded var to str literal
    cmd = "ls & cat /etc/passwd"
    subprocess.run(cmd, shell=True) 
    
    # hard-coded var to list of str literals
    cmd = ["ls", ";cat", "/etc/passwd"]
    subprocess.run(cmd, shell=True) 
    

    All reasonable enough .... but where do we draw the line? How about this case

    def main(cmd):
        subprocess.run(cmd, shell=True) 
    
    command = ["ls", ";cat", "/etc/passwd"]
    main(command)
    
    

    or even more app-like

    def app(user_input):
        subprocess.run(user_input) 
    

    I'm not sure either semgrep or libcst are able to distinguish these scenarios. At a certain point, a user-provided string is in fact a string literal.

  2. drdavella commented on Mar 5, 2024

    @drdavella
    ContributorAuthor

    @clavedeluna I agree with all but the last of your test cases; we should exclude those from modification if possible. We are going to be limited by the capabilities of libCST and Semgrep but even accounting for half of these will help reduce false positives.

    At a certain point, a user-provided string is in fact a string literal.

    I disagree; I mean "string literal" as a string that is actually defined within the codebase itself and not provided as user input. I'm not sure whether there is a better term of art to use here.

  3. drdavella commented on Mar 12, 2024

    @drdavella
    ContributorAuthor

    Just noting here that we can try using constant propagation in Semgrep to detect hardcoded string arguments. We'll need to determine which arguments are relevant and which are not.

  4. self-assigned this
    on Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions