Implement `flake8-bandit` #1646

charliermarsh · 2023-01-05T00:09:24Z

charliermarsh · 2023-01-05T00:12:12Z

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

edgarrmondragon · 2023-01-05T00:34:58Z

thanks @charliermarsh!

charliermarsh · 2023-01-05T00:36:50Z

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:

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).
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 · 2023-01-05T00:40:21Z

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:

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).

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.

…shlib`) (#1661) ref: #1646

See: #1646. Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

…#1692) ref: #1646 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

…e`) (#1695) ref: #1646

ahmedbilal · 2023-01-10T08:58:43Z

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

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

charliermarsh · 2023-01-10T13:02:49Z

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

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

…` (snmp weak cryptography) (#1771) ref: #1646 Co-authored-by: messense <messense@icloud.com> Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

ref: #1646 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

colin99d · 2023-01-12T18:29:16Z

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

colin99d · 2023-01-12T18:30:10Z

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 · 2023-01-12T19:54:02Z

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

…listen`) (#2108) ref: #1646

ref: astral-sh#1646

ruff has reimplemented the bandit rules[1], so we can use that as a better-integrated tool with the rest of our stack. We enable all the bandit rules and selectively disable some across the codebase and some in just tests where they don't make sense (e.g. flagging use of `assert` or using insecure crypto). For bonus points we can get rid of the GitPython dependency, which has a history of (non-exploitable in our context) security issues. [1] Per astral-sh/ruff#1646 they've implemented nearly all of them, and the remaining ones aren't that important IMO.

qdegraaf · 2023-11-06T14:44:12Z

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.

See: #1646.

See: #1646. Bandit origin: https://github.com/PyCQA/bandit/blob/main/bandit/plugins/tarfile_unsafe_members.py

ischaojie · 2023-11-26T08:10:51Z

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

pygaiwan · 2023-12-14T23:18:54Z

@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 · 2023-12-15T07:24:39Z

@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 · 2024-01-03T18:19:30Z

Five rules left :)

## Summary Adds all `S4XX` rules to the [flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin port. There is a lot of documentation to write, some tests can be expanded and implementation can probably be refactored to be more compact. As there is some discussion on whether this is actually useful. (See: #1646 (comment)), wanted to check which rules we want to have before I go through the process of polishing this up. ## Test Plan Fixtures for all rules based on `flake8-bandit` [tests](https://github.com/tylerwince/flake8-bandit/tree/main/tests) ## Issue link Refers: #1646

## Summary Adds `S504` rule for the [flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin port. Checks for calls to `ssl.wrap_socket` which have no `ssl_version` argument set. See also https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_no_version ## Test Plan Fixture added ## Issue Link Refers: #1646

## Summary Adds S502 rule for the [flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin port. Checks for calls to any function with keywords arguments `ssl_version` or `method` or for kwargs `method` in calls to `OpenSSL.SSL.Context` and `ssl_version` in calls to `ssl.wrap_socket` which have an insecure ssl_version valu. See also https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_bad_version ## Test Plan Fixture added ## Issue Link Refers: #1646

## Summary Adds S503 rule for the [flake8-bandit](https://github.com/tylerwince/flake8-bandit) plugin port. Checks for function defs argument defaults which have an insecure ssl_version value. See also https://bandit.readthedocs.io/en/latest/_modules/bandit/plugins/insecure_ssl_tls.html#ssl_with_bad_defaults Some logic and the `const` can be shared with #9390. When one of the two is merged. ## Test Plan Fixture added ## Issue Link Refers: #1646

edgarrmondragon · 2024-01-13T00:20:08Z

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

glennmatthews · 2024-01-16T15:15:10Z

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

itdependsnetworks · 2024-01-23T21:30:55Z

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.

Part of #1646. ## Summary Implement `S610` rule from `flake8-bandit`. Upstream references: - Implementation: https://github.com/PyCQA/bandit/blob/1.7.8/bandit/plugins/django_sql_injection.py#L20-L97 - Test cases: https://github.com/PyCQA/bandit/blob/1.7.8/examples/django_sql_injection_extra.py - Test assertion: https://github.com/PyCQA/bandit/blob/1.7.8/tests/functional/test_functional.py#L517-L524 The implementation in `bandit` targets additional arguments (`params`, `order_by` and `select_params`) but doesn't seem to do anything with them in the end, so I did not include them in the implementation. Note that this rule could be prone to false positives, as ideally we would want to check if `extra()` is tied to a [Django queryset](https://docs.djangoproject.com/en/5.0/ref/models/querysets/), but AFAIK Ruff is not able to resolve classes outside of the current module. ## Test Plan Snapshot tests

charliermarsh added the plugin Implementing a known but unsupported plugin label Jan 5, 2023

edgarrmondragon mentioned this issue Jan 5, 2023

Implement flake8-bandit rule S108 #1644

Merged

saadmk11 mentioned this issue Jan 5, 2023

[flake8-bandit] Add Rule for S324 (Insecure hash functions in hashlib) #1661

Merged

charliermarsh pushed a commit that referenced this issue Jan 5, 2023

[flake8-bandit] Add Rule for S324 (Insecure hash functions in `ha…

9f8ef17

…shlib`) (#1661) ref: #1646

charliermarsh mentioned this issue Jan 5, 2023

[flake8-bandit] Add Rule for S506 (unsafe use of yaml load) #1664

Merged

charliermarsh added a commit that referenced this issue Jan 5, 2023

[flake8-bandit] Add Rule for S506 (unsafe use of yaml load) (#1664)

2d23b1a

See: #1646. Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

saadmk11 mentioned this issue Jan 6, 2023

[flake8-bandit] Add Rule for S113 (requests call without timeout) #1692

Merged

charliermarsh added a commit that referenced this issue Jan 6, 2023

[flake8-bandit] Add Rule for S113 (requests call without timeout) (…

6aba43a

…#1692) ref: #1646 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

saadmk11 mentioned this issue Jan 6, 2023

[flake8-bandit] Add Rule for S501 (request call with verify=False) #1695

Merged

charliermarsh pushed a commit that referenced this issue Jan 6, 2023

[flake8-bandit] Add Rule for S501 (request call with `verify=Fals…

9409b49

…e`) (#1695) ref: #1646

saadmk11 mentioned this issue Jan 10, 2023

[flake8-bandit] Add Rule for S508 (snmp insecure version) & S509 (snmp weak cryptography) #1771

Merged

saadmk11 mentioned this issue Jan 12, 2023

[flake8-bandit] Add Rule for S701 (jinja2 autoescape false) #1815

Merged

charliermarsh added a commit that referenced this issue Jan 12, 2023

[flake8-bandit] Add Rule for S701 (jinja2 autoescape false) (#1815)

1a90408

ref: #1646 Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>

charliermarsh pinned this issue Jan 14, 2023

charliermarsh unpinned this issue Jan 14, 2023

saadmk11 mentioned this issue Jan 23, 2023

[flake8-bandit] Added Rule S612 (Use of insecure logging.config.listen) #2108

Merged

charliermarsh pushed a commit that referenced this issue Jan 23, 2023

[flake8-bandit] Added Rule S612 (Use of insecure `logging.config.…

8001a16

…listen`) (#2108) ref: #1646

sciyoshi added a commit to sciyoshi/ruff that referenced this issue Jan 26, 2023

[flake8-bandit] Added Rule S110 (try/except/pass)

c40fbae

ref: astral-sh#1646

sciyoshi mentioned this issue Jan 26, 2023

[flake8-bandit] Add Rule S110 (try/except/pass) #2197

Merged

sciyoshi added a commit to sciyoshi/ruff that referenced this issue Jan 26, 2023

[flake8-bandit] Added Rule S110 (try/except/pass)

ddc1985

ref: astral-sh#1646

legoktm mentioned this issue Sep 29, 2023

Replace bandit with ruff freedomofpress/securedrop#6961

Merged

3 tasks

ischaojie mentioned this issue Nov 7, 2023

[flake8-bandit] Implement mako-templates (S702) #8533

Merged

charliermarsh pushed a commit that referenced this issue Nov 7, 2023

[flake8-bandit] Implement mako-templates (S702) (#8533)

fce9f63

See: #1646.

hussein-awala mentioned this issue Nov 9, 2023

Add bandit to pre-commit to detect common security issues apache/airflow#34247

Merged

ischaojie mentioned this issue Nov 13, 2023

[flake8-bandit] Implement django-raw-sql (S611) #8651

Merged

charliermarsh pushed a commit that referenced this issue Nov 20, 2023

[flake8-bandit] Implement django-raw-sql (S611) (#8651)

653e51a

See: #1646.

ischaojie mentioned this issue Nov 23, 2023

[flake8-bandit] Implement tarfile-unsafe-members (S202) #8829

Merged

qdegraaf mentioned this issue Nov 24, 2023

[flake8-bandit] Implement S4XX suspicious import rules #8831

Merged

charliermarsh pushed a commit that referenced this issue Nov 24, 2023

[flake8-bandit] Implement tarfile-unsafe-members (S202) (#8829)

2590aa3

See: #1646. Bandit origin: https://github.com/PyCQA/bandit/blob/main/bandit/plugins/tarfile_unsafe_members.py

qdegraaf mentioned this issue Jan 3, 2024

[flake8-bandit] Add S504 SslWithNoVersion rule #9384

Merged

This was referenced Jan 4, 2024

[flake8-bandit] Implement S502 SslInsecureVersion rule #9390

Merged

[flake8-bandit] Implement S503 SslWithBadDefaults rule #9391

Merged

itdependsnetworks mentioned this issue Jan 8, 2024

Replace black, flake8, pydocstyle, bandit with ruff nautobot/nautobot-app-ssot#314

Closed

ashrub-holvi mentioned this issue Feb 2, 2024

S308 (mark_safe) doesn't detects decorator usage and imports from another place #9780

Closed

mkniewallner mentioned this issue Mar 9, 2024

[flake8-bandit]: Implement S610 rule #10316

Merged

JoseRZapata mentioned this issue Mar 25, 2024

pre-commit replace bandit with ruff JoseRZapata/data-science-project-template#45

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `flake8-bandit` #1646

Implement `flake8-bandit` #1646

charliermarsh commented Jan 5, 2023 •

edited

charliermarsh commented Jan 5, 2023

edgarrmondragon commented Jan 5, 2023

charliermarsh commented Jan 5, 2023

edgarrmondragon commented Jan 5, 2023

ahmedbilal commented Jan 10, 2023

charliermarsh commented Jan 10, 2023

colin99d commented Jan 12, 2023

colin99d commented Jan 12, 2023

charliermarsh commented Jan 12, 2023

qdegraaf commented Nov 6, 2023

ischaojie commented Nov 26, 2023

pygaiwan commented Dec 14, 2023

Daverball commented Dec 15, 2023

charliermarsh commented Jan 3, 2024

edgarrmondragon commented Jan 13, 2024

glennmatthews commented Jan 16, 2024

itdependsnetworks commented Jan 23, 2024

Implement flake8-bandit #1646

Implement flake8-bandit #1646

Comments

charliermarsh commented Jan 5, 2023 • edited

charliermarsh commented Jan 5, 2023

edgarrmondragon commented Jan 5, 2023

charliermarsh commented Jan 5, 2023

edgarrmondragon commented Jan 5, 2023

ahmedbilal commented Jan 10, 2023

charliermarsh commented Jan 10, 2023

colin99d commented Jan 12, 2023

colin99d commented Jan 12, 2023

charliermarsh commented Jan 12, 2023

qdegraaf commented Nov 6, 2023

ischaojie commented Nov 26, 2023

pygaiwan commented Dec 14, 2023

Daverball commented Dec 15, 2023

charliermarsh commented Jan 3, 2024

edgarrmondragon commented Jan 13, 2024

glennmatthews commented Jan 16, 2024

itdependsnetworks commented Jan 23, 2024

Implement `flake8-bandit` #1646

Implement `flake8-bandit` #1646

charliermarsh commented Jan 5, 2023 •

edited