# nosec with bandit ID doesn't work properly sometimes

ericwb commented 9 months ago

Describe the bug

Using nosec with a bandit ID like # nosec: B108 doesn't appear to always work. See reproduction steps.

Reproduction steps

1. Run .tox/py312/bin/bandit bandit/plugins/general_hardcoded_tmp.py
2. Notice you get the following issues:

Test results:
>> Issue: [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
   Severity: Medium   Confidence: Medium
   CWE: CWE-377 (https://cwe.mitre.org/data/definitions/377.html)
   More Info: https://bandit.readthedocs.io/en/1.7.7.dev6/plugins/b108_hardcoded_tmp_directory.html
   Location: bandit/plugins/general_hardcoded_tmp.py:62:29
61      if name == "hardcoded_tmp_directory":
62          return {"tmp_dirs": ["/tmp", "/var/tmp", "/dev/shm"]}
63  

--------------------------------------------------
>> Issue: [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
   Severity: Medium   Confidence: Medium
   CWE: CWE-377 (https://cwe.mitre.org/data/definitions/377.html)
   More Info: https://bandit.readthedocs.io/en/1.7.7.dev6/plugins/b108_hardcoded_tmp_directory.html
   Location: bandit/plugins/general_hardcoded_tmp.py:62:37
61      if name == "hardcoded_tmp_directory":
62          return {"tmp_dirs": ["/tmp", "/var/tmp", "/dev/shm"]}
63  

--------------------------------------------------
>> Issue: [B108:hardcoded_tmp_directory] Probable insecure usage of temp file/directory.
   Severity: Medium   Confidence: Medium
   CWE: CWE-377 (https://cwe.mitre.org/data/definitions/377.html)
   More Info: https://bandit.readthedocs.io/en/1.7.7.dev6/plugins/b108_hardcoded_tmp_directory.html
   Location: bandit/plugins/general_hardcoded_tmp.py:62:49
61      if name == "hardcoded_tmp_directory":
62          return {"tmp_dirs": ["/tmp", "/var/tmp", "/dev/shm"]}
63

Now add a trailing comment of # nosec: B108

Notice now you get a warning that B108 was not found on that line.

[tester]    WARNING nosec encountered (B108), but no failed test on line 62

Expected behavior

The nosec should not trigger a warning that the issue wasn't found.

Bandit version

1.7.6 (Default)

Python version

3.12 (Default)

Additional context

I suspect the issue is the algorithm in how tester.py:run_tests() is determining whether a test is skipped or not. If it finds no issue for any test ID, it then falls over to the warning message. But this doesn't always happen depending on the order of tests it iterates over and the ID to be skipped.

lukehinds commented 9 months ago

ok to close now @ericwb ?

ericwb commented 9 months ago

ok to close now @ericwb ?

No, it's still an issue. The example I gave, still shows the warning in the logs.

ArcturusMengsk commented 5 months ago

I have the same issue. My code looks approximately like this:

205  parameter = {  # nosec B108
206          "certificate": "/tmp/certificates/tls.crt",
207          "privateKey": "/tmp/certificates/tls.key",
208          "trustedCertificates": "/tmp/certificates/ca.crt",
209          "alias": "alias",
210  }

Regardless of which of these lines I put the # nosec on (any of lines 205-210), or how many of them I have (used to have one each on lines 206-208), I get the following warnings:

[tester]    WARNING nosec encountered (B108), but no failed test on line 206
[tester]    WARNING nosec encountered (B108), but no failed test on line 207
[tester]    WARNING nosec encountered (B108), but no failed test on line 208
[tester]    WARNING nosec encountered (B108), but no failed test on line 209
[tester]    WARNING nosec encountered (B108), but no failed test on line 209

Also note the double entry for line 209, which in fact doesn't even have the issue.

If I remove the # nosec, then Bandit fails with B108.

ericwb commented 5 months ago

In the example I gave, it actually is functioning as you'd expect. The line:

        return {"tmp_dirs": ["/tmp", "/var/tmp", "/dev/shm"]}  # nosec: B108

The plugin hardcoded_tmp_directory will be called 4 times. One time for each str object as this plugin is designed to check string literals for a strings ["/tmp", "/var/tmp", "/dev/shm"] in them. However the first string encountered "tmp_dirs" is not found to have one of those and it is label with a nosec (for the entire line). So the warning is output to the logs as a result.

A more ideal solution would be do nosec processing by line, not by test result encounter since you can have multiple strings per line obviously.

PyCQA / bandit