Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

# nosec with bandit ID doesn't work properly sometimes #1092

Open
ericwb opened this issue Jan 14, 2024 · 4 comments
Open

# nosec with bandit ID doesn't work properly sometimes #1092

ericwb opened this issue Jan 14, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@ericwb
Copy link
Member

ericwb commented Jan 14, 2024

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	
  1. Now add a trailing comment of # nosec: B108
  2. 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.

@ericwb ericwb added the bug Something isn't working label Jan 14, 2024
ericwb added a commit to ericwb/bandit that referenced this issue Jan 14, 2024
* Used nosec for false various positives
* Switched to usage of defusedxml
* Fixed the empty try-except-pass to have code in the except
  block.

Fixes PyCQA#1092

Signed-off-by: Eric Brown <[email protected]>
ericwb added a commit to ericwb/bandit that referenced this issue Jan 14, 2024
* Used nosec for false various positives.
  1. xml.etree is used only for XML generation not parsing
  2. "0.0.0.0" is used in the plugin itself
  3. Various strings of temp directories are used in the plugin
     itself.
  4. The subprocess call does use user input, but only from
     the command line itself that is running baseline. Although
     maybe this could be argued as an issue though.
* Fixed the empty try-except-pass to have code in the except
  block.

Fixes PyCQA#1092

Signed-off-by: Eric Brown <[email protected]>
@lukehinds
Copy link
Member

ok to close now @ericwb ?

@ericwb
Copy link
Member Author

ericwb commented Jan 21, 2024

ok to close now @ericwb ?

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

@ArcturusMengsk
Copy link

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
Copy link
Member Author

ericwb commented May 17, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants