Repository: PyCQA/bandit
Pull request #1111 (2024-02-22 21:50:10)

filter data is safe for tarfile extractall
Related to issue #1038

Currently the following line: `tarfile.extractall(path=some_path, filter="data")`  raises an error. See comment https://github.com/PyCQA/bandit/issues/1038#issuecomment-1956512283

However, this should be safe according to comment https://github.com/PyCQA/bandit/issues/1038#issuecomment-1834110126

This PR does not attempt to fix issue #1038, but starts by making the line aforementioned legal. If `filter="data"` is detected, the rule is early exited.

cc @mattiasb

Closes: #1025


etienneschalk: Hello, thanks for your review!
I remove the diff noise from formatting and rebases / squashed the commits.

To test locally I used 

```bash
bandit tests/functional/test_functional.py 
```

Patch:
diff --git a/bandit/plugins/tarfile_unsafe_members.py b/bandit/plugins/tarfile_unsafe_members.py
index 32c1e61..5ad145c 100644
--- a/bandit/plugins/tarfile_unsafe_members.py
+++ b/bandit/plugins/tarfile_unsafe_members.py
@@ -42,6 +42,9 @@ unless you explicitly need them.
 
 .. versionadded:: 1.7.5
 
+.. versionchanged:: 1.7.8
+    Added check for filter parameter
+
 """
 import ast
 
@@ -91,6 +94,13 @@ def get_members_value(context):
                 return {"Other": value}
 
 
+def is_filter_data(context):
+    for keyword in context.node.keywords:
+        if keyword.arg == "filter":
+            arg = keyword.value
+            return isinstance(arg, ast.Str) and arg.s == "data"
+
+
 @test.test_id("B202")
 @test.checks("Call")
 def tarfile_unsafe_members(context):
@@ -100,6 +110,8 @@ def tarfile_unsafe_members(context):
             "extractall" in context.call_function_name,
         ]
     ):
+        if "filter" in context.call_keywords and is_filter_data(context):
+            return None
         if "members" in context.call_keywords:
             members = get_members_value(context)
             if "Function" in members:
diff --git a/examples/tarfile_extractall.py b/examples/tarfile_extractall.py
index 2af3eb5..b32736a 100644
--- a/examples/tarfile_extractall.py
+++ b/examples/tarfile_extractall.py
@@ -15,6 +15,18 @@ def managed_members_archive_handler(filename):
     tar.close()
 
 
+def filter_data_archive_handler(filename):
+    tar = tarfile.open(filename)
+    tar.extractall(path=tempfile.mkdtemp(), filter="data")
+    tar.close()
+
+
+def filter_fully_trusted_archive_handler(filename):
+    tar = tarfile.open(filename)
+    tar.extractall(path=tempfile.mkdtemp(), filter="fully_trusted")
+    tar.close()
+
+
 def list_members_archive_handler(filename):
     tar = tarfile.open(filename)
     tar.extractall(path=tempfile.mkdtemp(), members=[])
@@ -45,3 +57,5 @@ if __name__ == "__main__":
         filename = sys.argv[1]
         unsafe_archive_handler(filename)
         managed_members_archive_handler(filename)
+        filter_data_archive_handler(filename)
+        filter_fully_trusted_archive_handler(filename)


Test patch:
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index a230dc3..401c46f 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -926,7 +926,7 @@ class FunctionalTests(testtools.TestCase):
     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},
+            "SEVERITY": {"UNDEFINED": 0, "LOW": 1, "MEDIUM": 2, "HIGH": 2},
+            "CONFIDENCE": {"UNDEFINED": 0, "LOW": 1, "MEDIUM": 2, "HIGH": 2},
         }
         self.check_example("tarfile_extractall.py", expect)