From: Andrei Pavel Date: Tue, 7 May 2024 10:28:48 +0000 (+0300) Subject: [#3287] fix bandit warnings X-Git-Tag: Kea-2.7.0~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0acd781c927e3fe5fa7e2e37226329e31daf7f3;p=thirdparty%2Fkea.git [#3287] fix bandit warnings --- diff --git a/doc/sphinx/api2doc.py b/doc/sphinx/api2doc.py index 527e453cbd..1f0a0ed21e 100755 --- a/doc/sphinx/api2doc.py +++ b/doc/sphinx/api2doc.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 -# Copyright (C) 2019-2023 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2019-2024 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this @@ -41,7 +41,9 @@ def read_input_files(files): except: print('\nError while processing %s\n\n' % f) raise - assert name == descr['name'] + if name != descr['name']: + exit("Expected name == descr['name'], but name is {name} and descr['name'] is {descr['name']}") + apis[name] = descr return apis diff --git a/hammer.py b/hammer.py index 7d72d56070..7d930f572b 100755 --- a/hammer.py +++ b/hammer.py @@ -24,22 +24,20 @@ import binascii import argparse import textwrap import functools -import subprocess import multiprocessing import grp import pwd import getpass +import urllib.request +from urllib.parse import urljoin -try: - import urllib.request -except: - pass -try: - from urllib.parse import urljoin -except: - from urlparse import urljoin +# [B404:blacklist] Consider possible security implications associated with subprocess module. +import subprocess # nosec B404 -import xml.etree.ElementTree as ET +# Issue: [B405:blacklist] Using xml.etree.ElementTree to parse untrusted XML data is known to be vulnerable to XML +# attacks. Replace xml.etree.ElementTree with the equivalent defusedxml package, or make sure +# defusedxml.defuse_stdlib() is called. +import xml.etree.ElementTree as ET # nosec B405 # SYSTEMS = { @@ -47,6 +45,8 @@ import xml.etree.ElementTree as ET # 'version': True if supported else False, # ... # }, +# ... +# } SYSTEMS = { 'fedora': { @@ -381,11 +381,13 @@ def execute(cmd, timeout=60, cwd=None, env=None, raise_error=True, dry_run=False for attempt in range(attempts): if interactive: - p = subprocess.Popen(cmd, cwd=cwd, env=env, shell=True) + # Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue. + p = subprocess.Popen(cmd, cwd=cwd, env=env, shell=True) # nosec B602 exitcode = p.wait() else: - p = subprocess.Popen(cmd, cwd=cwd, env=env, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + # Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security issue. + p = subprocess.Popen(cmd, cwd=cwd, env=env, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) # nosec B602 if capture: output = '' @@ -692,7 +694,10 @@ class VagrantEnv(object): return {} url = 'https://app.vagrantup.com/api/v1/box/' + (image_tpl if image_tpl else self.image_tpl) try: - with urllib.request.urlopen(url) as response: + # Issue: [B310:blacklist] Audit url open for permitted schemes. + # Allowing use of file:/ or custom schemes is often unexpected. + # Reason for nosec: it is clearly a https link. + with urllib.request.urlopen(url) as response: # nosec B310 data = response.read() except: log.exception('ignored exception') @@ -897,7 +902,8 @@ class VagrantEnv(object): if upload: repo_url = _get_full_repo_url(repository_url, self.system, self.revision, pkg_version) - assert repo_url is not None + if repo_url is None: + raise ValueError('repo_url is None') upload_cmd = 'curl -v --netrc -f' if self.system in ['ubuntu', 'debian']: @@ -2414,7 +2420,8 @@ def _build_native_pkg(system, revision, features, tarball_path, env, check_times env = _prepare_ccache_if_needed(system, ccache_dir, env) repo_url = _get_full_repo_url(repository_url, system, revision, pkg_version) - assert repo_url is not None + if repo_url is None: + raise ValueError('repo_url is None') if system in ['fedora', 'centos', 'rhel', 'rocky']: _build_rpm(system, revision, features, tarball_path, env, check_times, dry_run, @@ -2832,7 +2839,10 @@ def destroy_system(path): def _coin_toss(): - if random.randint(0, 65535) % 2 == 0: + # Issue: [B311:blacklist] Standard pseudo-random generators are not suitable for security/cryptographic + # purposes. + # Reason for nosec: It is not used in a security context. + if random.randint(0, 65535) % 2 == 0: # nosec B311 return True return False @@ -2977,7 +2987,8 @@ def upload_to_repo(args, pkgs_dir): # NOTE: note the differences (if any) in system/revision vs args.system/revision system, revision = get_system_revision() repo_url = _get_full_repo_url(args.repository_url, system, revision, args.pkg_version) - assert repo_url is not None + if repo_url is None: + raise ValueError('repo_url is None') upload_cmd = 'curl -v --netrc -f' log.info('args.system %s, system = %s', args.system, system) diff --git a/src/bin/shell/kea_connector3.py b/src/bin/shell/kea_connector3.py index c1d4f5ca64..a14fb2618f 100644 --- a/src/bin/shell/kea_connector3.py +++ b/src/bin/shell/kea_connector3.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2021 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2017-2024 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this @@ -20,6 +20,8 @@ def send_to_control_agent(params): # First, create the URL url = params.scheme + "://" + params.http_host + ":" url += str(params.http_port) + str(params.path) + if not url.lower().startswith('http'): + raise ValueError(f"url {url} is not an http link") # Now prepare the request (URL, headers and body) req = urllib.request.Request(url=url, @@ -41,7 +43,10 @@ def send_to_control_agent(params): ssl_ctx.load_cert_chain(params.cert, params.key) # Establish connection, send the request. - resp = urllib.request.urlopen(req, context=ssl_ctx) + # Issue: [B310:blacklist] Audit url open for permitted schemes. + # Allowing use of file:/ or custom schemes is often unexpected. + # Reason for nosec: url is checked to be http further above. + resp = urllib.request.urlopen(req, context=ssl_ctx) # nosec B310 # Now get the response details, put it in CAResponse and return it result = CAResponse(resp.getcode(), resp.reason, diff --git a/src/share/database/scripts/utils/are-scripts-in-sync.py b/src/share/database/scripts/utils/are-scripts-in-sync.py index 399ef56a2b..4ac33e0cfa 100755 --- a/src/share/database/scripts/utils/are-scripts-in-sync.py +++ b/src/share/database/scripts/utils/are-scripts-in-sync.py @@ -8,9 +8,11 @@ import difflib import glob import os import re -import subprocess import sys +# [B404:blacklist] Consider possible security implications associated with subprocess module. +import subprocess # nosec B404 + def usage(): print('''\ @@ -185,7 +187,9 @@ def execute(command): ''' if 'DEBUG' in os.environ: print(f'> {command}') - with subprocess.Popen(command, encoding='utf-8', shell=True, + # Issue: [B602:subprocess_popen_with_shell_equals_true] subprocess call with shell=True identified, security + # issue. + with subprocess.Popen(command, encoding='utf-8', shell=True, # nosec B602 stdout=subprocess.PIPE, stderr=subprocess.PIPE) as p: output, error = p.communicate() if error: diff --git a/tools/git-obsolete-branch.py b/tools/git-obsolete-branch.py index 4c5826e596..c9e7dc66a7 100755 --- a/tools/git-obsolete-branch.py +++ b/tools/git-obsolete-branch.py @@ -1,6 +1,6 @@ #!/usr/bin/python # -# Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2012-2024 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this @@ -24,10 +24,13 @@ # tomek import string -import subprocess import sys from optparse import OptionParser +# [B404:blacklist] Consider possible security implications associated with subprocess module. +import subprocess # nosec B404 + + class Branch: MERGED = 1 NOTMERGED = 2 @@ -42,7 +45,7 @@ def branch_list_get(verbose): if all changes on that branch are also on master. """ # call git branch -r (list of remote branches) - txt_list = subprocess.check_output(["git", "branch", "-r"]) + txt_list = check_output(["git", "branch", "-r"]) txt_list = txt_list.split(b"\n") @@ -73,8 +76,9 @@ def branch_list_get(verbose): # get a diff with changes that are on that branch only # i.e. all unmerged code. + # Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input. cmd = ["git", "diff", "master..." + branch_info.name ] - diff = subprocess.check_output(cmd) + diff = check_output(cmd) if len(diff) == 0: # No diff? Then all changes from that branch are on master as well. branch_info.status = Branch.MERGED @@ -84,7 +88,8 @@ def branch_list_get(verbose): # %ai = date, %ae = author e-mail, %an = author name cmd = [ "git" , "log", "-n", "1", "--pretty=\"%ai,%ae,%an\"", branch_info.name ] - offender = subprocess.check_output(cmd) + # Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input. + offender = check_output(cmd) offender = offender.strip(b"\n\"") # comment out this 2 lines to disable obfuscation @@ -146,6 +151,11 @@ def branch_print(branches, csv, print_merged, print_notmerged, print_stats): print("#Not merged: %d" % notmerged) +def check_output(cmd): + # Issue: [B603:subprocess_without_shell_equals_true] subprocess call - check for execution of untrusted input. + return subprocess.check_output(cmd) # nosec B603 + + def parse_args(args=sys.argv[1:], Parser=OptionParser): parser = Parser(description="This script prints out merged and/or unmerged"