Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
- ran into an interesting pylint bug, if you have methods on a class that don't access self, pylint throws an exception and returns an exit code of 8
- update README to include documentation about nosec comments
  • Loading branch information
mikespallino committed Apr 7, 2020
1 parent e2d7cd2 commit 8948bef
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 58 deletions.
17 changes: 17 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,23 @@ security issue, it will not be reported::

self.process = subprocess.Popen('/bin/echo', shell=True) # nosec

Because multiple issues can be reported for the same line, specific tests may
be provided to suppress those reports. This will cause other issues not
included to be reported. This can be useful in preventing situations where a
nosec comment is used, but a separate vulnerability may be added to the line
later causing the new vulnerability to be ignored.

For example, this will suppress the report of B602 and B607::

self.process = subprocess.Popen('/bin/ls *', shell=True) #nosec B602, B607

Full test names rather than the test ID may also be used.

For example, this will suppress the report of B101 and continue to report B506
as an issue.::

assert yaml.load("{}") == [] # nosec assert_used


Vulnerability Tests
-------------------
Expand Down
121 changes: 63 additions & 58 deletions bandit/core/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@


LOG = logging.getLogger(__name__)
NOSEC_SPECIFIC_TEST_IDS_IGNORE = re.compile(r'([bB][\d]+),?[ ]?')
NOSEC_SPECIFIC_TEST_NAMES_IGNORE = re.compile(r'([a-z_]+),?[ ]?')
NOSEC_SPECIFIC_IDS_IGNORE = re.compile(r"([bB][\d]+),?[ ]?")
NOSEC_SPECIFIC_NAMES_IGNORE = re.compile(r"([a-z_]+),?[ ]?")


class BanditManager(object):
Expand Down Expand Up @@ -260,60 +260,6 @@ def run_tests(self):
# do final aggregation of metrics
self.metrics.aggregate()

def _parse_nosec_comment(self, comment):
nosec_no_space = '#nosec'
nosec_space = '# nosec'
nospace = comment.find(nosec_no_space)
space = comment.find(nosec_space)
if nospace > -1:
start_comment = nospace
nosec_length = len(nosec_no_space)
elif space > -1:
start_comment = space
nosec_length = len(nosec_space)
else:
# None is explicitly different to empty set, None indicates no
# nosec comment on a line
return None

# find the next # separator
end = comment.find('#', start_comment + 1)
# if we don't find one, set end index to the length
if end == -1:
end = len(comment)

# extract text after #[ ]?nosec and the end index, this is just the
# list of potential test ids or names
nosec_comment = comment[start_comment+nosec_length:end]

# empty set indicates that there was a nosec comment without specific
# test ids or names
test_ids = set()
if nosec_comment:
extman = extension_loader.MANAGER
# check for IDS to ignore (e.g. B602)
for test in re.finditer(NOSEC_SPECIFIC_TEST_IDS_IGNORE,
nosec_comment):
test_id_match = test.group(1)
plugin_id = extman.check_id(test_id_match)
if plugin_id:
test_ids.add(test_id_match)
else:
LOG.warning("Test in comment: %s is not a test id",
test_id_match)
# check for NAMES to ignore (e.g. assert_used)
for test in re.finditer(NOSEC_SPECIFIC_TEST_NAMES_IGNORE,
nosec_comment):
plugin_id = extman.get_plugin_id(test.group(1))
if plugin_id:
test_ids.add(plugin_id)
else:
LOG.warning(
"Test in comment: %s is not a test name, ignoring",
test.group(1))

return test_ids

def _parse_file(self, fname, fdata, new_files_list):
try:
# parse the current file
Expand All @@ -334,8 +280,7 @@ def _parse_file(self, fname, fdata, new_files_list):
if not self.ignore_nosec:
for toktype, tokval, (lineno, _), _, _ in tokens:
if toktype == tokenize.COMMENT:
nosec_lines[lineno] = self._parse_nosec_comment(
tokval)
nosec_lines[lineno] = _parse_nosec_comment(tokval)

except tokenize.TokenError:
pass
Expand Down Expand Up @@ -463,3 +408,63 @@ def _find_candidate_matches(unmatched_issues, results_list):
unmatched == i])

return issue_candidates


def _get_tests_from_nosec_comment(comment):
nosec_no_space = '#nosec'
nosec_space = '# nosec'
nospace = comment.find(nosec_no_space)
space = comment.find(nosec_space)
if nospace > -1:
start_comment = nospace
nosec_length = len(nosec_no_space)
elif space > -1:
start_comment = space
nosec_length = len(nosec_space)
else:
# None is explicitly different to empty set, None indicates no
# nosec comment on a line
return None

# find the next # separator
end = comment.find('#', start_comment + 1)
# if we don't find one, set end index to the length
if end == -1:
end = len(comment)

# extract text after #[ ]?nosec and the end index, this is just the
# list of potential test ids or names
return comment[start_comment + nosec_length:end]


def _parse_nosec_comment(comment):
nosec_tests = _get_tests_from_nosec_comment(comment)
if nosec_tests is None:
return None
# empty set indicates that there was a nosec comment without specific
# test ids or names
test_ids = set()
if nosec_tests:
extman = extension_loader.MANAGER
# check for IDS to ignore (e.g. B602)
for test in re.finditer(NOSEC_SPECIFIC_IDS_IGNORE,
nosec_tests):
test_id_match = test.group(1)
plugin_id = extman.check_id(test_id_match)
if plugin_id:
test_ids.add(test_id_match)
else:
LOG.warning("Test in comment: %s is not a test id",
test_id_match)
# check for NAMES to ignore (e.g. assert_used)
for test in re.finditer(NOSEC_SPECIFIC_NAMES_IGNORE,
nosec_tests):
plugin_id = extman.get_plugin_id(test.group(1))
if plugin_id:
test_ids.add(plugin_id)
else:
LOG.warning(
"Test in comment: %s is not a test name, ignoring",
test.group(1))

return test_ids

0 comments on commit 8948bef

Please sign in to comment.