-
-
Notifications
You must be signed in to change notification settings - Fork 585
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
Updates to address docstring code scan issues, add flake8 configuration #671
Changes from 6 commits
08ae28a
dabff1a
e9468cc
487d799
810b037
6d6974b
604faac
0f2486a
b72faa3
984575e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
[flake8] | ||
max-line-length=120 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,16 @@ | ||
#!/usr/bin/env python | ||
# SPDX-License-Identifier: Apache-2.0 | ||
"""Bandit is a tool designed to find common security issues in Python code. | ||
|
||
Bandit is a tool designed to find common security issues in Python code. | ||
To do this Bandit processes each file, builds an AST from it, and runs appropriate | ||
plugins against the AST nodes. Once Bandit has finished scanning all the files | ||
it generates a report. | ||
|
||
Bandit was originally developed within the OpenStack Security Project and | ||
later rehomed to PyCQA. | ||
|
||
https://bandit.readthedocs.io/ | ||
""" | ||
from bandit.cli import main | ||
main.main() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,8 @@ | |
# Copyright 2016 Hewlett-Packard Development Company, L.P. | ||
# | ||
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
r""" | ||
==================================================== | ||
Blacklist various Python calls known to be dangerous | ||
==================================================== | ||
Blacklist various Python calls known to be dangerous. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By removing the '===', this changes the font from a heading to normal paragraph text. However, this text is intended to be a heading. See https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverting first line missing period warning update. |
||
|
||
This blacklist data checks for a number of Python calls known to have possible | ||
security implications. The following blacklist tests are run against any | ||
|
@@ -329,7 +326,6 @@ def gen_blacklist(): | |
|
||
:return: a dictionary mapping node types to a list of blacklist data | ||
""" | ||
|
||
sets = [] | ||
sets.append(utils.build_conf_dict( | ||
'pickle', 'B301', | ||
|
@@ -346,12 +342,12 @@ def gen_blacklist(): | |
'shelve.DbfilenameShelf'], | ||
'Pickle and modules that wrap it can be unsafe when used to ' | ||
'deserialize untrusted data, possible security issue.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'marshal', 'B302', ['marshal.load', 'marshal.loads'], | ||
'Deserialization with the marshal module is possibly dangerous.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'md5', 'B303', | ||
|
@@ -368,7 +364,7 @@ def gen_blacklist(): | |
'cryptography.hazmat.primitives.hashes.MD5', | ||
'cryptography.hazmat.primitives.hashes.SHA1'], | ||
'Use of insecure MD2, MD4, MD5, or SHA1 hash function.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'ciphers', 'B304', | ||
|
@@ -388,30 +384,30 @@ def gen_blacklist(): | |
'Use of insecure cipher {name}. Replace with a known secure' | ||
' cipher such as AES.', | ||
'HIGH' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'cipher_modes', 'B305', | ||
['cryptography.hazmat.primitives.ciphers.modes.ECB'], | ||
'Use of insecure cipher mode {name}.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'mktemp_q', 'B306', ['tempfile.mktemp'], | ||
'Use of insecure and deprecated function (mktemp).' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'eval', 'B307', ['eval'], | ||
'Use of possibly insecure function - consider using safer ' | ||
'ast.literal_eval.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'mark_safe', 'B308', ['django.utils.safestring.mark_safe'], | ||
'Use of mark_safe() may expose cross-site scripting ' | ||
'vulnerabilities and should be reviewed.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'httpsconnection', 'B309', | ||
|
@@ -421,7 +417,7 @@ def gen_blacklist(): | |
'Use of HTTPSConnection on older versions of Python prior to 2.7.9 ' | ||
'and 3.4.3 do not provide security, see ' | ||
'https://wiki.openstack.org/wiki/OSSN/OSSN-0033' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'urllib_urlopen', 'B310', | ||
|
@@ -441,7 +437,7 @@ def gen_blacklist(): | |
'six.moves.urllib.request.FancyURLopener'], | ||
'Audit url open for permitted schemes. Allowing use of file:/ or ' | ||
'custom schemes is often unexpected.' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'random', 'B311', | ||
|
@@ -454,14 +450,14 @@ def gen_blacklist(): | |
'Standard pseudo-random generators are not suitable for ' | ||
'security/cryptographic purposes.', | ||
'LOW' | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'telnetlib', 'B312', ['telnetlib.*'], | ||
'Telnet-related functions are being called. Telnet is considered ' | ||
'insecure. Use SSH or some other encrypted protocol.', | ||
'HIGH' | ||
)) | ||
)) | ||
|
||
# Most of this is based off of Christian Heimes' work on defusedxml: | ||
# https://pypi.org/project/defusedxml/#defusedxml-sax | ||
|
@@ -478,7 +474,7 @@ def gen_blacklist(): | |
'xml.etree.cElementTree.fromstring', | ||
'xml.etree.cElementTree.XMLParser'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_ElementTree', 'B314', | ||
|
@@ -487,41 +483,41 @@ def gen_blacklist(): | |
'xml.etree.ElementTree.fromstring', | ||
'xml.etree.ElementTree.XMLParser'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_expatreader', 'B315', ['xml.sax.expatreader.create_parser'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_expatbuilder', 'B316', | ||
['xml.dom.expatbuilder.parse', | ||
'xml.dom.expatbuilder.parseString'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_sax', 'B317', | ||
['xml.sax.parse', | ||
'xml.sax.parseString', | ||
'xml.sax.make_parser'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_minidom', 'B318', | ||
['xml.dom.minidom.parse', | ||
'xml.dom.minidom.parseString'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_pulldom', 'B319', | ||
['xml.dom.pulldom.parse', | ||
'xml.dom.pulldom.parseString'], | ||
xml_msg | ||
)) | ||
)) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'xml_bad_etree', 'B320', | ||
|
@@ -534,7 +530,7 @@ def gen_blacklist(): | |
('Using {name} to parse untrusted XML data is known to be ' | ||
'vulnerable to XML attacks. Replace {name} with its ' | ||
'defusedxml equivalent function.') | ||
)) | ||
)) | ||
|
||
# end of XML tests | ||
|
||
|
@@ -543,7 +539,7 @@ def gen_blacklist(): | |
'FTP-related functions are being called. FTP is considered ' | ||
'insecure. Use SSH/SFTP/SCP or some other encrypted protocol.', | ||
'HIGH' | ||
)) | ||
)) | ||
|
||
# skipped B322 as the check for a call to input() has been removed | ||
|
||
|
@@ -554,14 +550,14 @@ def gen_blacklist(): | |
'using an insecure context via the _create_unverified_context that ' | ||
'reverts to the previous behavior that does not validate certificates ' | ||
'or perform hostname checks.' | ||
)) | ||
)) | ||
|
||
# skipped B324 (used in bandit/plugins/hashlib_new_insecure_functions.py) | ||
|
||
sets.append(utils.build_conf_dict( | ||
'tempnam', 'B325', ['os.tempnam', 'os.tmpnam'], | ||
'Use of os.tempnam() and os.tmpnam() is vulnerable to symlink ' | ||
'attacks. Consider using tmpfile() instead.' | ||
)) | ||
)) | ||
|
||
return {'Call': sets} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,7 @@ | |
# SPDX-License-Identifier: Apache-2.0 | ||
|
||
r""" | ||
====================================================== | ||
Blacklist various Python imports known to be dangerous | ||
====================================================== | ||
Blacklist various Python imports known to be dangerous. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By removing the '===', this changes the font from a heading to normal paragraph text. However, this text is intended to be a heading. See https://bandit.readthedocs.io/en/latest/blacklists/blacklist_calls.html There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reverting first line missing period warning update. |
||
|
||
This blacklist data checks for a number of Python modules known to have | ||
possible security implications. The following blacklist tests are run against | ||
|
@@ -215,7 +213,6 @@ | |
+------+---------------------+------------------------------------+-----------+ | ||
|
||
""" | ||
|
||
from bandit.blacklists import utils | ||
|
||
|
||
|
@@ -230,7 +227,6 @@ def gen_blacklist(): | |
|
||
:return: a dictionary mapping node types to a list of blacklist data | ||
""" | ||
|
||
sets = [] | ||
sets.append(utils.build_conf_dict( | ||
'import_telnetlib', 'B401', ['telnetlib'], | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost all of the codebase is already conforming to a 80 char max line length (inherited from checks when in OpenStack). Even though 80 can be short at times, I think it would be best to keep the pattern. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverting this check. 119 is the limit for Github PR's, will stay with OpenStack convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if line too long warning should be ignored individually for those single-line docstring comments, or should the docstring be split up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should line wrap those lines that are 80 or more. I already merged this commit, so feel free to create a new PR to handle above.