CI: Configure bandit better

Declare the tests to skip, rather than the tests to run. This ensures
that we pick up new bandit checks automatically.

I recently noticed a use of md5() without the usedforsecurity=False
kwarg. Confused about why this wasn't caught in the gate, I eventually
traced it back to B303 (which we explicitly enabled) being largely
superseded by B324 (which did not exist when we wrote down the tests
to enable).

Flag a bunch of false-positives with "# nosec" comments, resolve two
other errors, and skip some more-pervasive errors, to be resolved later.

Change-Id: Ia054e4f7c9e5bf29064a66933e27830adbc107d3
This commit is contained in:
Tim Burke 2024-12-06 12:21:01 -08:00
parent a55a48ffc8
commit e576c5cee0
8 changed files with 33 additions and 16 deletions

View File

@ -79,10 +79,28 @@
# B703 : django_mark_safe # B703 : django_mark_safe
# (optional) list included test IDs here, eg '[B101, B406]': # (optional) list included test IDs here, eg '[B101, B406]':
tests: [B102, B103, B302, B303, B304, B305, B306, B307, B308, B310, B401, B501, B502, B506, B601, B602, B609] tests:
# (optional) list skipped test IDs here, eg '[B101, B406]': # (optional) list skipped test IDs here, eg '[B101, B406]':
skips: skips:
# We default to binding to all interfaces
- B104
# Yes, we sometimes catch just to quietly swallow an exception
- B110
# We use insecure randomness all over the place, because
# it's exceedingly rare that we need secure randomness
- B311
# We dynamically build SQL all over the place
- B608
# We often use subprocesses, and require a lot of trust in our use of them
- B404
- B603
- B607
# We parse xml
- B320
- B405
- B410
- B603
### (optional) plugin settings - some test plugins require configuration data ### (optional) plugin settings - some test plugins require configuration data
### that may be given here, per-plugin. All bandit test plugins have a built in ### that may be given here, per-plugin. All bandit test plugins have a built in

View File

@ -17,7 +17,6 @@
from __future__ import print_function from __future__ import print_function
import os import os
import sys import sys
from hashlib import md5
import getopt import getopt
from itertools import chain from itertools import chain
@ -28,6 +27,7 @@ from six.moves.urllib.parse import quote
from swift.common.ring import Ring from swift.common.ring import Ring
from swift.common.utils import split_path from swift.common.utils import split_path
from swift.common.utils.base import md5
from swift.common.bufferedhttp import http_connect from swift.common.bufferedhttp import http_connect
@ -91,7 +91,7 @@ class Auditor(object):
conn = http_connect(node['ip'], node['port'], conn = http_connect(node['ip'], node['port'],
node['device'], part, 'GET', path, {}) node['device'], part, 'GET', path, {})
resp = conn.getresponse() resp = conn.getresponse()
calc_hash = md5() calc_hash = md5(usedforsecurity=False)
chunk = True chunk = True
while chunk: while chunk:
chunk = resp.read(8192) chunk = resp.read(8192)

View File

@ -197,7 +197,7 @@ payload sent to the proxy (the list of objects/containers to be deleted).
import json import json
import six import six
import tarfile import tarfile
from xml.sax import saxutils from xml.sax.saxutils import escape # nosec B406
from time import time from time import time
from eventlet import sleep from eventlet import sleep
import zlib import zlib
@ -247,14 +247,14 @@ def get_response_body(data_format, data_dict, error_list, root_tag):
xml_key = key.replace(' ', '_').lower() xml_key = key.replace(' ', '_').lower()
output.extend([ output.extend([
'<', xml_key, '>', '<', xml_key, '>',
saxutils.escape(str(data_dict[key])), escape(str(data_dict[key])),
'</', xml_key, '>\n', '</', xml_key, '>\n',
]) ])
output.append('<errors>\n') output.append('<errors>\n')
for name, status in error_list: for name, status in error_list:
output.extend([ output.extend([
'<object><name>', saxutils.escape(name), '</name><status>', '<object><name>', escape(name), '</name><status>',
saxutils.escape(status), '</status></object>\n', escape(status), '</status></object>\n',
]) ])
output.extend(['</errors>\n</', root_tag, '>\n']) output.extend(['</errors>\n</', root_tag, '>\n'])
if six.PY2: if six.PY2:

View File

@ -97,7 +97,7 @@ from swift.common.middleware.x_profile.html_viewer import HTMLViewer
from swift.common.middleware.x_profile.profile_model import ProfileLog from swift.common.middleware.x_profile.profile_model import ProfileLog
DEFAULT_PROFILE_PREFIX = '/tmp/log/swift/profile/default.profile' DEFAULT_PROFILE_PREFIX = '/tmp/log/swift/profile/default.profile' # nosec B108
# unwind the iterator; it may call start_response, do lots of work, etc # unwind the iterator; it may call start_response, do lots of work, etc
PROFILE_EXEC_EAGER = """ PROFILE_EXEC_EAGER = """

View File

@ -730,7 +730,7 @@ class Accept(object):
""" """
# RFC 2616 section 2.2 # RFC 2616 section 2.2
token = r'[^()<>@,;:\"/\[\]?={}\x00-\x20\x7f]+' token = r'[^()<>@,;:\"/\[\]?={}\x00-\x20\x7f]+' # nosec B105
qdtext = r'[^"]' qdtext = r'[^"]'
quoted_pair = r'(?:\\.)' quoted_pair = r'(?:\\.)'
quoted_string = r'"(?:' + qdtext + r'|' + quoted_pair + r')*"' quoted_string = r'"(?:' + qdtext + r'|' + quoted_pair + r')*"'

View File

@ -2854,7 +2854,7 @@ def friendly_close(resp):
return drain_and_close(resp, read_limit=DEFAULT_DRAIN_LIMIT) return drain_and_close(resp, read_limit=DEFAULT_DRAIN_LIMIT)
_rfc_token = r'[^()<>@,;:\"/\[\]?={}\x00-\x20\x7f]+' _rfc_token = r'[^()<>@,;:\"/\[\]?={}\x00-\x20\x7f]+' # nosec B105
_rfc_extension_pattern = re.compile( _rfc_extension_pattern = re.compile(
r'(?:\s*;\s*(' + _rfc_token + r")\s*(?:=\s*(" + _rfc_token + r'(?:\s*;\s*(' + _rfc_token + r")\s*(?:=\s*(" + _rfc_token +
r'|"(?:[^"\\]|\\.)*"))?)') r'|"(?:[^"\\]|\\.)*"))?)')

View File

@ -1026,7 +1026,7 @@ def run_wsgi(conf_path, app_section, *args, **kwargs):
if hasattr(os, 'set_inheritable'): if hasattr(os, 'set_inheritable'):
# See https://www.python.org/dev/peps/pep-0446/ # See https://www.python.org/dev/peps/pep-0446/
os.set_inheritable(write_fd, True) os.set_inheritable(write_fd, True)
os.execv(myself, sys.argv) os.execv(myself, sys.argv) # nosec B606
logger.error('Somehow lived past os.execv()?!') logger.error('Somehow lived past os.execv()?!')
exit('Somehow lived past os.execv()?!') exit('Somehow lived past os.execv()?!')
elif child_pid == 0: elif child_pid == 0:

View File

@ -1087,11 +1087,10 @@ class BaseDiskFileManager(object):
key = info_key[:-5] + '_file' key = info_key[:-5] + '_file'
results[key] = join(datadir, info['filename']) if info else None results[key] = join(datadir, info['filename']) if info else None
if verify: if verify and not self._verify_ondisk_files(results, **kwargs):
assert self._verify_ondisk_files( raise RuntimeError(
results, **kwargs), \ "On-disk file search algorithm contract is broken: %s"
"On-disk file search algorithm contract is broken: %s" \ % str(results))
% str(results)
return results return results