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

Updates to address docstring code scan issues, add flake8 configuration #671

Merged
merged 10 commits into from
Dec 20, 2020
2 changes: 2 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[flake8]
max-line-length=120
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

bandit\formatters\screen.py:98:80: E501 line too long (99 > 79 characters)
    """Output issue str returns a list of lines that should be added to the existing lines list."""
                                                                               ^
bandit\formatters\text.py:44:80: E501 line too long (103 > 79 characters)
    """Get verbose details including files in scope, score, SEVERITY and CONFLUENCE, files excluded."""
                                                                               ^
bandit\formatters\text.py:71:80: E501 line too long (99 > 79 characters)
    """Output issue str returns a list of lines that should be added to the existing lines list."""

Copy link
Member

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.

13 changes: 13 additions & 0 deletions bandit/__main__.py
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()
48 changes: 23 additions & 25 deletions bandit/blacklists/calls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
# Copyright 2016 Hewlett-Packard Development Company, L.P.
#
# SPDX-License-Identifier: Apache-2.0

r"""
====================================================
Blacklist various Python calls known to be dangerous
Expand Down Expand Up @@ -329,7 +328,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',
Expand All @@ -346,12 +344,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',
Expand All @@ -368,7 +366,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',
Expand All @@ -388,30 +386,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',
Expand All @@ -421,7 +419,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',
Expand All @@ -441,7 +439,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',
Expand All @@ -454,14 +452,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
Expand All @@ -478,7 +476,7 @@ def gen_blacklist():
'xml.etree.cElementTree.fromstring',
'xml.etree.cElementTree.XMLParser'],
xml_msg
))
))

sets.append(utils.build_conf_dict(
'xml_bad_ElementTree', 'B314',
Expand All @@ -487,41 +485,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',
Expand All @@ -534,7 +532,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

Expand All @@ -543,7 +541,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

Expand All @@ -554,14 +552,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}
2 changes: 0 additions & 2 deletions bandit/blacklists/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@
+------+---------------------+------------------------------------+-----------+

"""

from bandit.blacklists import utils


Expand All @@ -230,7 +229,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'],
Expand Down
2 changes: 1 addition & 1 deletion bandit/blacklists/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
# Copyright 2016 Hewlett-Packard Development Company, L.P.
#
# SPDX-License-Identifier: Apache-2.0
r"""Utils module."""


def build_conf_dict(name, bid, qualnames, message, level='MEDIUM'):
"""Build and return a blacklist configuration dict."""

return {'name': name, 'id': bid, 'message': message,
'qualnames': qualnames, 'level': level}
9 changes: 9 additions & 0 deletions bandit/cli/baseline.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
# reports on any new findings.
# #############################################################################

"""Bandit is a tool designed to find common security issues in Python code."""


import argparse
import contextlib
import logging
Expand All @@ -33,8 +36,11 @@
report_basename = 'bandit_baseline_result'
valid_baseline_formats = ['txt', 'html', 'json']

"""baseline.py"""


def main():
"""Execute Bandit."""
# our cleanup function needs this and can't be passed arguments
global current_commit
global repo
Expand Down Expand Up @@ -120,6 +126,7 @@ def main():
# #################### Clean up before exit ###################################
@contextlib.contextmanager
def baseline_setup():
"""Baseline setup by creating temp folder and resetting repo."""
d = tempfile.mkdtemp()
yield d
shutil.rmtree(d, True)
Expand All @@ -130,6 +137,7 @@ def baseline_setup():

# #################### Setup logging ##########################################
def init_logger():
"""Init logger."""
LOG.handlers = []
log_level = logging.INFO
log_format_string = "[%(levelname)7s ] %(message)s"
Expand All @@ -142,6 +150,7 @@ def init_logger():

# #################### Perform initialization and validate assumptions ########
def initialize():
"""Initialize arguments and output formats."""
valid = True

# #################### Parse Args #########################################
Expand Down
7 changes: 7 additions & 0 deletions bandit/cli/config_generator.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Copyright 2015 Red Hat Inc.
#
# SPDX-License-Identifier: Apache-2.0
"""Bandit is a tool designed to find common security issues in Python code."""


from __future__ import print_function

import argparse
Expand Down Expand Up @@ -49,6 +52,7 @@


def init_logger():
"""Init logger."""
LOG.handlers = []
log_level = logging.INFO
log_format_string = "[%(levelname)5s]: %(message)s"
Expand All @@ -60,6 +64,7 @@ def init_logger():


def parse_args():
"""Parse arguments."""
help_description = """Bandit Config Generator

This tool is used to generate an optional profile. The profile may be used
Expand Down Expand Up @@ -100,6 +105,7 @@ def parse_args():


def get_config_settings():
"""Get configuration settings."""
config = {}
for plugin in extension_loader.MANAGER.plugins:
fn_name = plugin.name
Expand All @@ -117,6 +123,7 @@ def get_config_settings():


def main():
"""Config generator to write configuration file."""
init_logger()
args = parse_args()

Expand Down
8 changes: 6 additions & 2 deletions bandit/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
# Copyright 2014 Hewlett-Packard Development Company, L.P.
#
# SPDX-License-Identifier: Apache-2.0
"""Bandit is a tool designed to find common security issues in Python code."""


import argparse
import fnmatch
import logging
Expand All @@ -21,11 +24,11 @@


def _init_logger(log_level=logging.INFO, log_format=None):
'''Initialize the logger
"""Initialize the logger.

:param debug: Whether to enable debug mode
:return: An instantiated logging instance
'''
"""
LOG.handlers = []

if not log_format:
Expand Down Expand Up @@ -120,6 +123,7 @@ def _log_info(args, profile):


def main():
"""Bandit CLI."""
# bring our logging stuff up as early as possible
debug = (logging.DEBUG if '-d' in sys.argv or '--debug' in sys.argv else
logging.INFO)
Expand Down