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

Using # nosec BXXX annotation in a nested dict causes "higher" annotations to be ignored #1003

Open
0xDEC0DE opened this issue Mar 23, 2023 · 4 comments · May be fixed by #1004
Open

Using # nosec BXXX annotation in a nested dict causes "higher" annotations to be ignored #1003

0xDEC0DE opened this issue Mar 23, 2023 · 4 comments · May be fixed by #1004
Labels
bug Something isn't working

Comments

@0xDEC0DE
Copy link

0xDEC0DE commented Mar 23, 2023

Describe the bug

Using a # nosec BXXX annotation inside a nested data structure appears to cause "higher" nosec annotations to be ignored:

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.5/plugins/b108_hardcoded_tmp_directory.html
   Location: derp.py:7:23
6	    ),
7	    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
8	    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
9	}

--------------------------------------------------
>> 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.5/plugins/b108_hardcoded_tmp_directory.html
   Location: derp.py:8:25
7	    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
8	    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
9	}

--------------------------------------------------

Reproduction steps

Use this as a test case (save to testcase.py):

example = {
    'S3_CONFIG_PARAMS': dict(  # nosec B106
        aws_access_key_id='key_goes_here',
        aws_secret_access_key='secret_goes_here',
        endpoint_url='s3.amazonaws.com',
    ),
    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
}

..and run bandit testcase.py

Expected behavior

0 issues found.

Bandit version

1.7.5

Python version

3.11.2

Additional context

  1. Using plain # nosec annotations works.
  2. This test case also works, but throws warnings:
example = {
    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
    'S3_CONFIG_PARAMS': dict(
        aws_access_key_id='key_goes_here',
        aws_secret_access_key='secret_goes_here',  # nosec B106
        endpoint_url='s3.amazonaws.com',
    ),
}

So the issue would appear to have something to do with ignoring individual tests, and nesting depth.

@0xDEC0DE 0xDEC0DE added the bug Something isn't working label Mar 23, 2023
@kfrydel
Copy link
Contributor

kfrydel commented Mar 24, 2023

@0xDEC0DE Is it regression or 1.7.4 has it as well?

@0xDEC0DE
Copy link
Author

@0xDEC0DE Is it regression or 1.7.4 has it as well?

This appears to be a regression; 1.7.4 ignores them (albeit with warnings):

Setup

python3 -m venv 1.7.4
python3 -m venv 1.7.5
1.7.4/bin/pip install bandit==1.7.4
1.7.5/bin/pip install bandit==1.7.5
# double-check install
1.7.4/bin/pip freeze
1.7.5/bin/pip freeze

1.7.4

Note the warnings from tester

$ 1.7.4/bin/bandit testcase.py
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.11.2
[node_visitor]	WARNING	Unable to find qualified name for module: testcase.py
[tester]	WARNING	nosec encountered (B108), but no failed test on line 7
[tester]	WARNING	nosec encountered (B108), but no failed test on line 8
Run started:2023-03-24 16:00:56.967967

Test results:
	No issues identified.

1.7.5

$ 1.7.5/bin/bandit testcase.py
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.11.2
[node_visitor]	WARNING	Unable to find qualified name for module: testcase.py
Run started:2023-03-24 16:00:58.901676

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.5/plugins/b108_hardcoded_tmp_directory.html
   Location: testcase.py:7:23
6	    ),
7	    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
8	    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
9	}

--------------------------------------------------
>> 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.5/plugins/b108_hardcoded_tmp_directory.html
   Location: testcase.py:8:25
7	    'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
8	    'ALPINE_APORTS_DIR': '/tmp/derp',  # nosec B108
9	}

--------------------------------------------------

kfrydel added a commit to kfrydel/bandit that referenced this issue Mar 27, 2023
Before this commit nosec was searched from the begnning
of the expression's context, which may be broader than
the exact piece of code that a developer wants to skip.
This caused, that for the below example:

1. example = {
2.     'S3_CONFIG_PARAMS': dict(  # nosec B106
3.         ...
4.     ),
5.     'LOCALFS_BASEDIR': '/var/tmp/herp',  # nosec B108
6. }

for line 5, nosec from line 2 was returned. Thus `nosec B108` was ignored.

This commit changes the algorithm that search for nosec for an expression
and nosec from the exact line of the expression is preferred.

Resolves: PyCQA#1003
@kfrydel kfrydel linked a pull request Mar 27, 2023 that will close this issue
@kfrydel
Copy link
Contributor

kfrydel commented Mar 27, 2023

@0xDEC0DE Could you check if the linked pull request (#1004) solves your issue?

I think my previous PR introduced the regression: #915

@0xDEC0DE
Copy link
Author

I included a test case so that you could tell me 😆

But yes, #1004 appears to report no issues, but it DOES throw warnings, same as 1.7.4:

$ bandit testcase.py
[main]	INFO	profile include tests: None
[main]	INFO	profile exclude tests: None
[main]	INFO	cli include tests: None
[main]	INFO	cli exclude tests: None
[main]	INFO	running on Python 3.11.2
[node_visitor]	WARNING	Unable to find qualified name for module: testcase.py
[tester]	WARNING	nosec encountered (B108), but no failed test on line 7
[tester]	WARNING	nosec encountered (B108), but no failed test on line 8
Run started:2023-03-28 16:56:18.401826

Test results:
	No issues identified.

Code scanned:
	Total lines of code: 9
	Total lines skipped (#nosec): 0

Run metrics:
	Total issues (by severity):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
	Total issues (by confidence):
		Undefined: 0
		Low: 0
		Medium: 0
		High: 0
Files skipped (0):

rkuczer added a commit to rkuczer/bandit that referenced this issue Apr 4, 2023
rkuczer added a commit to rkuczer/bandit that referenced this issue Apr 6, 2023
rkuczer added a commit to rkuczer/bandit that referenced this issue Apr 6, 2023
rkuczer added a commit to rkuczer/bandit that referenced this issue Apr 7, 2023
rkuczer added a commit to rkuczer/bandit that referenced this issue Apr 7, 2023
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

Successfully merging a pull request may close this issue.

2 participants