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

Adding tarfile.extractall() plugin with examples #549

Merged
merged 22 commits into from Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b6268f9
Adding tarfile.extractall() plugin with examples
yilmi-vmw Nov 5, 2019
143ab0a
Merge branch 'master' into tarfile
ericwb Jan 12, 2020
90126cb
Merge branch 'master' into tarfile
ericwb Jan 21, 2020
b03ba99
Update README.rst
ericwb Jul 11, 2022
0ceb4aa
Apply suggestions from code review
ericwb Jul 11, 2022
c1fc587
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
9aa702e
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
edb857e
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
ecef381
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
bf5a32c
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
6703472
Update doc/source/plugins/b612_tarfile_unsafe_members.rst
ericwb Jul 11, 2022
4847e70
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
42dd4d5
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
a8f38bc
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
e1349fa
Update README.rst
ericwb Jul 11, 2022
d4261c8
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
c249502
Merge branch 'main' into tarfile
ericwb Jul 11, 2022
ff3f756
Apply suggestions from code review
ericwb Jul 11, 2022
4dc85a5
Update tarfile_unsafe_members.py
ericwb Jul 11, 2022
ea44418
Update tarfile_unsafe_members.py
ericwb Jul 11, 2022
1d45173
Update bandit/plugins/tarfile_unsafe_members.py
ericwb Jul 11, 2022
959c9b5
Update test_functional.py
ericwb Jul 11, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 109 additions & 0 deletions bandit/plugins/tarfile_unsafe_members.py
@@ -0,0 +1,109 @@
#
# SPDX-License-Identifier: Apache-2.0
#
r"""
ericwb marked this conversation as resolved.
Show resolved Hide resolved
=================================
B202: Test for tarfile.extractall
=================================

This plugin will look for usage of ``tarfile.extractall()``

Severity are set as follows:

* ``tarfile.extractalll(members=function(tarfile))`` - LOW
* ``tarfile.extractalll(members=?)`` - member is not a function - MEDIUM
* ``tarfile.extractall()`` - members from the archive is trusted - HIGH

Use ``tarfile.extractall(members=function_name)`` and define a function
that will inspect each member. Discard files that contain a directory
traversal sequences such as ``../`` or ``\..`` along with all special filetypes
unless you explicitly need them.

:Example:

.. code-block:: none

>> Issue: [B202:tarfile_unsafe_members] tarfile.extractall used without
any validation. You should check members and discard dangerous ones
Severity: High Confidence: High
CWE: CWE-22 (https://cwe.mitre.org/data/definitions/22.html)
Location: examples/tarfile_extractall.py:8
More Info:
https://bandit.readthedocs.io/en/latest/plugins/b202_tarfile_unsafe_members.html
7 tar = tarfile.open(filename)
8 tar.extractall(path=tempfile.mkdtemp())
9 tar.close()


.. seealso::

- https://docs.python.org/3/library/tarfile.html#tarfile.TarFile.extractall
- https://docs.python.org/3/library/tarfile.html#tarfile.TarInfo

.. versionadded:: 1.7.5

"""
import ast

import bandit
from bandit.core import issue
from bandit.core import test_properties as test
ericwb marked this conversation as resolved.
Show resolved Hide resolved


def exec_issue(level, members=""):
if level == bandit.LOW:
return bandit.Issue(
severity=bandit.LOW,
confidence=bandit.LOW,
cwe=issue.Cwe.PATH_TRAVERSAL,
text="Usage of tarfile.extractall(members=function(tarfile)). "
"Make sure your function properly discards dangerous members "
"{members}).".format(members=members),
)
elif level == bandit.MEDIUM:
return bandit.Issue(
severity=bandit.MEDIUM,
confidence=bandit.MEDIUM,
cwe=issue.Cwe.PATH_TRAVERSAL,
text="Found tarfile.extractall(members=?) but couldn't "
"identify the type of members. "
"Check if the members were properly validated "
"{members}).".format(members=members),
)
else:
return bandit.Issue(
severity=bandit.HIGH,
confidence=bandit.HIGH,
cwe=issue.Cwe.PATH_TRAVERSAL,
text="tarfile.extractall used without any validation. "
"Please check and discard dangerous members.",
)


def get_members_value(context):
for keyword in context.node.keywords:
if keyword.arg == "members":
arg = keyword.value
if isinstance(arg, ast.Call):
return {"Function": arg.func.id}
else:
value = arg.id if isinstance(arg, ast.Name) else arg
return {"Other": value}


@test.test_id("B202")
@test.checks("Call")
def tarfile_unsafe_members(context):
if all(
[
context.is_module_imported_exact("tarfile"),
"extractall" in context.call_function_name,
]
):
if "members" in context.call_keywords:
members = get_members_value(context)
if "Function" in members:
return exec_issue(bandit.LOW, members)
else:
return exec_issue(bandit.MEDIUM, members)
return exec_issue(bandit.HIGH)
5 changes: 5 additions & 0 deletions doc/source/plugins/b612_tarfile_unsafe_members.rst
@@ -0,0 +1,5 @@
----------------------------
B202: tarfile_unsafe_members
----------------------------

.. automodule:: bandit.plugins.tarfile_unsafe_members
47 changes: 47 additions & 0 deletions examples/tarfile_extractall.py
@@ -0,0 +1,47 @@
import sys
import tarfile
import tempfile


def unsafe_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp())
tar.close()


def managed_members_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp(), members=members_filter(tar))
tar.close()


def list_members_archive_handler(filename):
tar = tarfile.open(filename)
tar.extractall(path=tempfile.mkdtemp(), members=[])
tar.close()


def provided_members_archive_handler(filename):
tar = tarfile.open(filename)
tarfile.extractall(path=tempfile.mkdtemp(), members=tar)
tar.close()


def members_filter(tarfile):
result = []
for member in tarfile.getmembers():
if '../' in member.name:
print('Member name container directory traversal sequence')
continue
elif (member.issym() or member.islnk()) and ('../' in member.linkname):
print('Symlink to external resource')
continue
result.append(member)
return result


if __name__ == "__main__":
if len(sys.argv) > 1:
filename = sys.argv[1]
unsafe_archive_handler(filename)
managed_members_archive_handler(filename)
3 changes: 3 additions & 0 deletions setup.cfg
Expand Up @@ -140,6 +140,9 @@ bandit.plugins =
# bandit/plugins/logging_config_insecure_listen.py
logging_config_insecure_listen = bandit.plugins.logging_config_insecure_listen:logging_config_insecure_listen

#bandit/plugins/tarfile_unsafe_members.py
tarfile_unsafe_members = bandit.plugins.tarfile_unsafe_members:tarfile_unsafe_members

[build_sphinx]
all_files = 1
build-dir = doc/build
Expand Down
8 changes: 8 additions & 0 deletions tests/functional/test_functional.py
Expand Up @@ -904,3 +904,11 @@ def test_snmp_security_check(self):
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 0, "MEDIUM": 0, "HIGH": 3},
}
self.check_example("snmp.py", expect)

def test_tarfile_unsafe_members(self):
"""Test insecure usage of tarfile."""
expect = {
"SEVERITY": {"UNDEFINED": 0, "LOW": 1, "MEDIUM": 2, "HIGH": 1},
"CONFIDENCE": {"UNDEFINED": 0, "LOW": 1, "MEDIUM": 2, "HIGH": 1},
}
self.check_example("tarfile_extractall.py", expect)