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

Feature Request: Whitelist lines ending in # nosec #108

Closed
KevinHock opened this issue Apr 14, 2018 · 7 comments
Closed

Feature Request: Whitelist lines ending in # nosec #108

KevinHock opened this issue Apr 14, 2018 · 7 comments

Comments

@KevinHock
Copy link
Collaborator

KevinHock commented Apr 14, 2018

So both detect-secrets and Bandit have the concept of whitelisting a line by putting a comment at the end, similar to how you've probably seen people do # noqa: F401 or whatever, with pylint.

Let us steal once again, from Bandit, since they are most similar to us, here are the relevant lines, but we shall change lineno + 1 for to enumerate(lines, start=1) because it is more pythonic.

They also have the --ignore-nosec do not skip lines with # nosec comments command line optionso we shall pass in the set of lines to the 2 calls tofind_vulnerabilities` in __main__,

@KevinHock
Copy link
Collaborator Author

KevinHock commented Apr 14, 2018

While I initially thought we would just give the option to add # nosec to the sinks, e.g.

@app.route('/menu', methods=['POST'])
def menu():
    param = request.form['suggestion']
    command = 'echo ' + param + ' >> ' + 'menu.txt'

    subprocess.call(command, shell=True)  # nosec

    with open('menu.txt','r') as f:
        menu = f.read()

    return render_template('command_injection.html', menu=menu)

We can, without writing any more lines of code, give the option to add # nosec to the sources, e.g.

@app.route('/menu', methods=['POST'])
def menu():
    param = request.form['suggestion']  # nosec
    command = 'echo ' + param + ' >> ' + 'menu.txt'

    subprocess.call(command, shell=True)

    with open('menu.txt','r') as f:
        menu = f.read()

    return render_template('command_injection.html', menu=menu)

This may be the preference of a security team, as the code may change in the future, and it's more dangerous to have the sink be whitelisted, because there may be multiple vuln chains.

@KevinHock
Copy link
Collaborator Author

Would you like to take this one, @omergunal?

@omergunal
Copy link
Contributor

sure

@KevinHock KevinHock assigned KevinHock and unassigned KevinHock Apr 14, 2018
@KevinHock
Copy link
Collaborator Author

I forgot to say this before, but after passing the set of lines to find_vulnerabilities we can ignore both sources and sinks in the same function, find_triggers, something like if node.lineno not in nosec_lines:

@KevinHock
Copy link
Collaborator Author

Also for some reason I cannot 'Assign' you the issue in GitHub, maybe you can assign yourself.

@omergunal
Copy link
Contributor

I couldnt do

omergunal added a commit to omergunal/pyt that referenced this issue Apr 22, 2018
Added args and nosec_lines
@KevinHock
Copy link
Collaborator Author

Woohoo 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants