Adding tarfile.extractall() plugin with examples (#549)

* Adding tarfile.extractall() plugin with examples

* Update README.rst

* Apply suggestions from code review

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update doc/source/plugins/b612_tarfile_unsafe_members.rst

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update README.rst

* Update bandit/plugins/tarfile_unsafe_members.py

* Apply suggestions from code review

* Update tarfile_unsafe_members.py

* Update tarfile_unsafe_members.py

* Update bandit/plugins/tarfile_unsafe_members.py

* Update test_functional.py

Co-authored-by: Yassine Ilmi <yilmi@vmware.com>
Co-authored-by: Eric Brown <ericwb@users.noreply.github.com>

bandit/plugins/tarfile_unsafe_members.py

@@ -0,0 +1,109 @@
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+r"""
+=================================
+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
+
+
+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)

doc/source/plugins/b612_tarfile_unsafe_members.rst

@@ -0,0 +1,5 @@
+----------------------------
+B202: tarfile_unsafe_members
+----------------------------
+
+.. automodule:: bandit.plugins.tarfile_unsafe_members

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)

setup.cfg

@@ -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

tests/functional/test_functional.py

@@ -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)