PyCQA / bandit

Bandit is a tool designed to find common security issues in Python code.
https://bandit.readthedocs.io
Apache License 2.0
6.5k stars 610 forks source link

bandit does not consistently detect extractall with TarFile #1171

Open slavos1 opened 2 months ago

slavos1 commented 2 months ago

Describe the bug

Hello bandit team, I observed TarFile.extractall is not detected as vulnerable (B202:tarfile_unsafe_members) without explicit import tarfile line present (even if it is not actually used).

I created three simple files to demonstrate the issue:

  1. case1.py uses both TarFile.extractall and tarfile.extractall
  2. case2.py uses TarFile.extractall only
  3. case3.py uses TarFile.extractall only but with "useless" import tarfile
# bandit-repro/case1.py
import tarfile
from pathlib import Path
from tarfile import TarFile

def extractall_TarFile(p: Path, out: Path):
    "bandit detects this correctly as vulnerable"
    with TarFile(p) as tar:
        tar.extractall(out)

def extract_tarfile(p: Path, out: Path):
    "bandit detects this correctly as vulnerable"
    with tarfile.open(p) as tar:
        tar.extractall(out)
# bandit-repro/case2.py
from pathlib import Path
from tarfile import TarFile

def extractall_bandit_does_not_detect(p: Path, out: Path):
    "bandit *does not* detect this correctly as vulnerable"
    with TarFile(p) as tar:
        tar.extractall(out)
# bandit-repro/case3.py
import tarfile  # noqa: F401
from pathlib import Path
from tarfile import TarFile

def extractall_bandit_detects(p: Path, out: Path):
    "bandit *does* detect this correctly as vulnerable with surplus tarfile import"
    with TarFile(p) as tar:
        tar.extractall(out)

Then, when I ran bandit, it did not detect that the vulnerability is present in case2.py as well:

$ bandit -r -a file -f txt bandit-repro

Run started:2024-09-03 22:52:08.975866

Test results:
>> Issue: [B202:tarfile_unsafe_members] tarfile.extractall used without any validation. Please check and discard dangerous members.
   Severity: High   Confidence: High
   CWE: CWE-22 (https://cwe.mitre.org/data/definitions/22.html)
   More Info: https://bandit.readthedocs.io/en/1.7.9/plugins/b202_tarfile_unsafe_members.html
   Location: bandit-repro/case1.py:9:8
8       with TarFile(p) as tar:
9           tar.extractall(out)
10  

--------------------------------------------------
>> Issue: [B202:tarfile_unsafe_members] tarfile.extractall used without any validation. Please check and discard dangerous members.
   Severity: High   Confidence: High
   CWE: CWE-22 (https://cwe.mitre.org/data/definitions/22.html)
   More Info: https://bandit.readthedocs.io/en/1.7.9/plugins/b202_tarfile_unsafe_members.html
   Location: bandit-repro/case1.py:15:8
14      with tarfile.open(p) as tar:
15          tar.extractall(out)

--------------------------------------------------
>> Issue: [B202:tarfile_unsafe_members] tarfile.extractall used without any validation. Please check and discard dangerous members.
   Severity: High   Confidence: High
   CWE: CWE-22 (https://cwe.mitre.org/data/definitions/22.html)
   More Info: https://bandit.readthedocs.io/en/1.7.9/plugins/b202_tarfile_unsafe_members.html
   Location: bandit-repro/case3.py:9:8
8       with TarFile(p) as tar:
9           tar.extractall(out)

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

Code scanned:
    Total lines of code: 24
    Total lines skipped (#nosec): 0
    Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

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

Reproduction steps

1. pipx install bandit
2. create folder bandit-repro with case1.py, case2.py and case3.py (as shown above) in it
3. bandit -r -a file -f txt bandit-repro
4. observe case2.py in not in the report

Expected behavior

TarFile.extractall should be reported as vulnerable no matter if import tarfile is present.

Bandit version

1.7.9 (Default)

Python version

3.12 (Default)

Additional context

Thanks for the project!

slavos1 commented 2 months ago

Looking at the code, these lines confirm the behaviour -- explicit import tarfile must be present otherwise B202 will not be detected.

https://github.com/PyCQA/bandit/blob/4ac55dfaaacf2083497831294b2dcd7e679f8428/bandit/plugins/tarfile_unsafe_members.py#L107-L112