Implement flake8-bandit

[charliermarsh]

• S101: assert_used
• S102: exec_used
• S103: set_bad_file_permissions
• S104: hardcoded_bind_all_interfaces
• S105: hardcoded_password_string
• S106: hardcoded_password_funcarg
• S107: hardcoded_password_default
• S108: hardcoded_tmp_directory
• S109: password_config_option_not_marked_secret
• S110: try_except_pass
• S111: execute_with_run_as_root_equals_true
• S112: try_except_continue
• S113: request_without_timeout
• S201: flask_debug_true
• S202: tarfile_unsafe_members
• S301: pickle
• S302: marshal
• S303: md5
• S304: ciphers
• S305: cipher_modes
• S306: mktemp_q
• S307: eval
• S308: mark_safe
• S311: random
• S312: telnetlib
• S313: xml_bad_cElementTree
• S314: xml_bad_ElementTree
• S315: xml_bad_expatreader
• S316: xml_bad_expatbuilder
• S317: xml_bad_sax
• S318: xml_bad_minidom
• S319: xml_bad_pulldom
• S320: xml_bad_etree
• S321: ftplib
• S323: unverified_context
• S324: hashlib
• S310: urllib_urlopen
• S401: import_telnetlib
• S402: import_ftplib
• S403: import_pickle
• S404: import_subprocess
• S405: import_xml_etree
• S406: import_xml_sax
• S407: import_xml_expat
• S408: import_xml_minidom
• S409: import_xml_pulldom
• S410: import_lxml
• S411: import_xmlrpclib
• S412: import_httpoxy
• S413: import_pycrypto
• S415: import_pyghmi
• S501: request_with_no_cert_validation
• S502: ssl_with_bad_version
• S503: ssl_with_bad_defaults
• S504: ssl_with_no_version
• S505: weak_cryptographic_key
• S506: yaml_load
• S507: ssh_no_host_key_verification
• S508: snmp_insecure_version
• S509: snmp_weak_cryptography
• S601: paramiko_calls
• S602: subprocess_popen_with_shell_equals_true
• S603: subprocess_without_shell_equals_true
• S604: any_other_function_with_shell_equals_true
• S605: start_process_with_a_shell
• S606: start_process_with_no_shell
• S607: start_process_with_partial_path
• S608: hardcoded_sql_expressions
• S609: linux_commands_wildcard_injection
• S610: django_extra_used
• S611: django_rawsql_used
• S612: logging_config_insecure_listen
• S701: jinja2_autoescape_false
• S702: use_of_mako_templates
• S703: django_mark_safe

[charliermarsh]

\cc @edgarrmondragon - easier to track here than to keep incrementing the count in the README. I'll populate the checklist.

[edgarrmondragon]

thanks @charliermarsh!

[charliermarsh]

By the way: the scripts are pretty bad right now, but it might save you some time (and would love help improving):

python scripts/add_check.py --name ConvertLoopToAll --code SIM111 --plugin flake8-simplify

The main limitations right now are:

1. Doesn't keep the members sorted in registry.rs, so has to be manually resorted (else checks appear out-of-order in the docs and elsewhere).
2. Assumes you're using a single plugins.rs file, instead of individual files for each check or a checks.rs file or whatever else, so that also requires manual tweaks.

[edgarrmondragon]

By the way: the scripts are pretty bad right now, but it might save you some time (and would love help improving):

python scripts/add_check.py --name ConvertLoopToAll --code SIM111 --plugin flake8-simplify

The main limitations right now are:

1. Doesn't keep the members sorted in registry.rs, so has to be manually resorted (else checks appear out-of-order in the docs and elsewhere).
2. Assumes you're using a single plugins.rs file, instead of individual files for each check or a checks.rs file or whatever else, so that also requires manual tweaks.

Thanks, I'll take a look at those. They might be really helpful for bandit's blacklist tests.

[ahmedbilal]

Please implement the configurations for it as well e.g exclude_dirs.

https://bandit.readthedocs.io/en/latest/config.html#bandit-settings

[charliermarsh]

@ahmedbilal - I think exclude_dirs can be accomplished with the per-file-ignores settings, like:

[tool.ruff.per-file-ignores]
"excluded_dir" = ["S"]

[colin99d]

Not sure if this is the best place to ask, but do flake8-bandit and bandit have the same linters?

[colin99d]

Also @charliermarsh do you think it would be helpful to pin all of the issues that are trackers for implementing a package to the top? It would make it easier for people to understand our progress, and to contribute to one.

[charliermarsh]

Not sure if this is the best place to ask, but do flake8-bandit and bandit have the same linters?

Yeah, I believe flake8-bandit is just a wrapper around bandit to make it compatible with Flake8: https://github.com/tylerwince/flake8-bandit/blob/3200f00bf34a8ff131139add500db3e62ee1e7fd/flake8_bandit.py#L9

[qdegraaf]

I am also interested in picking up some of the leftover rules. Has any decision been made on the S4XX import rules yet? Also how does Ruff deal with Bandit's severities? E.g. if I wanted to implement S202 (source docs) is there a default severity level that Ruff ports? Or do we split it into 3 separate smaller rules?

Clicked through some ported rules and it looks like S103 has a med and high version and Ruff just ports both in the same rule but for something like S202 that might be a bit aggressive.

[ischaojie]

These rules have already been implemented:
S201: flask_debug_true
S601: paramiko_calls
S609: linux_commands_wildcard_injection

[pygaiwan]

@charliermarsh I think is more a generic question rather than specific to this rule set, but are you implementing them with the exact same behaviour of the originals?

I am getting the same (odd) behaviour in ruff and flake8

❯ ruff ruff_test.py
ruff_test.py:4:1: S311 Standard pseudo-random generators are not suitable for security/cryptographic purposes.

With this sample code:

import random
import time

time.sleep(random.choice(range(1, 5)))

I was wondering if this behaviour is going to be reviewed or fixed in Ruff? Thanks!

[Daverball]

@pygaiwan This is the intended behavior of that rule and not odd at all. The random module is not safe, if you care about security, since the random sequences are predictable. Whether or not you use random in a place where security matters is not the linters' job to figure out and also quite impossible to do reliably in a dynamic language like Python. There's a secrets module which contains some of the same functions as random but implemented in a more secure way.

Bandit is a security linter so its goal is to have essentially zero false negatives, at the cost of some false positives, so it's quite common that you will have to review bandit errors and decide whether or not it's an actual issue and put a noqa/nosec comment, when you decide it isn't a problem.

You can also globally disable rules you don't care about.

[charliermarsh]

Five rules left :)

[edgarrmondragon]

S503 was addressed by #9391 so there's only S610 and S703 left!

[glennmatthews]

How is S703 different from S308, by the way? They seem redundant to me.

[itdependsnetworks]

How is S703 different from S308, by the way? They seem redundant to me.

Per PyCQA/bandit#294 (comment), it would seem so. Would seem like S308 was first and then S703 covered S308 but did not want to have a breaking change.