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

Update calls.py with additional rulesets #637

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
61 changes: 61 additions & 0 deletions bandit/blacklists/calls.py
Expand Up @@ -569,4 +569,65 @@ def gen_blacklist():
'attacks. Consider using tmpfile() instead.'
))

# updated rulesets starts here

sets.append(utils.build_conf_dict(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a bandit plugin to check for use of yaml.load. See https://github.com/PyCQA/bandit/blob/master/bandit/plugins/yaml_load.py

'yaml_load', 'B326',
['yaml.load'],
"""
Use of this deprecated deserialize method leads to
remote code execution. See CVE-2017-18342.
"""
'Use yaml.safe_load instead.'
))

sets.append(utils.build_conf_dict(
'response_splitting', 'B327',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This CVE was resolved already in Python 3.7, 3.8, and greater. At this point, I think it's unnecessary for Bandit to flag that module.

[
'urllib2.Request.add_header',
'urllib.URLopener.addheader',
'urllib.URLopener.addheaders'
],
'Python Urllib and Urllib2 are vulnerable to CRLF injection.'
'See https://bugs.python.org/issue36276'
))

sets.append(utils.build_conf_dict(
'Base64_encoding_used', 'B328',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some very good use cases to use base64. In fact, some protocols require it. So there's a very low confidence this check would find a security issue. I'd rather not introduce many more false positives.

[
'base64.b64encode',
'base64.standard_b64encode',
'base64.urlsafe_b64encode',
'base64.b16encode',
'base64.b32encode',
'base64.encodestring',
'flask_scrypt.enbase64',
'base64.b64decode'
],
"""
Weak encoding alogirthm. Encoding and storing sensitive
information using this algorithm gives a false sense of security.
"""
))

sets.append(utils.build_conf_dict(
'Server_Side_Template_Injection', 'B329',
['render_template_string', 'render_template'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these part of Flask? If so, they would need to be fully qualified.

"""
User controlled Values injected into the template can lead to
arbitrary code executions in the server. Check if input values
are properly sanitized. See https://blog.nvisium.com/p263
"""
))

sets.append(utils.build_conf_dict(
'file_upload', 'B330',
['Flask.send_file'],
prabhuved marked this conversation as resolved.
Show resolved Hide resolved
"""
Presence of a file upload funtionality. Ensure that the
application whitelists the allowed file extensions and
validated the MIME type.'
"""
))

return {'Call': sets}