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 15 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions bandit/plugins/tarfile_unsafe_members.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
#
# 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
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
ericwb marked this conversation as resolved.
Show resolved Hide resolved

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


def exec_issue(level, members=''):
ericwb marked this conversation as resolved.
Show resolved Hide resolved
if level == bandit.LOW:
return bandit.Issue(
severity=bandit.LOW,
confidence=bandit.LOW,
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,
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,
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':
ericwb marked this conversation as resolved.
Show resolved Hide resolved
arg = keyword.value
if isinstance(arg, ast.Call):
return {'Function': arg.func.id}
ericwb marked this conversation as resolved.
Show resolved Hide resolved
else:
value = arg.id if isinstance(arg, ast.Name) else arg
return {'Other': value}
ericwb marked this conversation as resolved.
Show resolved Hide resolved


@test.test_id('B202')
@test.checks('Call')
ericwb marked this conversation as resolved.
Show resolved Hide resolved
def tarfile_unsafe_members(context):
if all([
context.is_module_imported_exact('tarfile'),
ericwb marked this conversation as resolved.
Show resolved Hide resolved
'extractall' in context.call_function_name]):
ericwb marked this conversation as resolved.
Show resolved Hide resolved
if 'members' in context.call_keywords:
ericwb marked this conversation as resolved.
Show resolved Hide resolved
members = get_members_value(context)
if 'Function' in members:
ericwb marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
----------------------------
B202: tarfile_unsafe_members
----------------------------

.. automodule:: bandit.plugins.tarfile_unsafe_members
47 changes: 47 additions & 0 deletions examples/tarfile_extractall.py
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ bandit.plugins =
# bandit/plugins/ssh_no_host_key_verification.py
ssh_no_host_key_verification = bandit.plugins.ssh_no_host_key_verification:ssh_no_host_key_verification

#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