Repository: PyCQA/bandit
Pull request #701 (2021-03-30 20:00:13)

#694 Bandit fails when using importlib with named arguments
This PR fixes the issue when importlib is being used with named arguments outlined in the #694. 

Following cases are found:

1. `importlib.import_module("foo", "bar.baz")` results in `call_args = ["foo", "bar.baz"]` and `call_keywords = {}`
2. `importlib.import_module("foo", package="bar.baz")` results in `call_args = ["foo"]`  and `call_keywords = {"package": "bar.baz"}`
3. `importlib.import_module(name="foo", package="bar.baz")` results in `call_args = []`  and `call_keywords = {"name": "foo", "package": "bar.baz"}`

The simplest approach is to use `context.call_args_count` and if the call_args_count is larger than 0, call `call_args[0]` which will choose `foo`. If call_args_count is equal to 0 then we need to choose `name` from the call_keywords.

I haven't found previous tests covering the blacklisting importlib functionality so if anyone more fluent in the codebase could point me in the direction of where this should be tested I can add the test cases for this functionality.

sigmavirus24: I think we run tests against the `examples` directory where you could add to https://github.com/PyCQA/bandit/blob/master/examples/imports-with-importlib.py
maciejstromich: @sigmavirus24 ah, that makes sense now. I've added two new examples to the import-with-imposelib.py. Thanks for pointing that out 
maciejstromich: @sigmavirus24 I've found one more interesting case. When these new tests are executed against master they are not failing the build because bandit throws an exception and continues with execution so I've updated the results to follow incremented values and changed the modules in the tests to trigger additional issues. Also I've updated tests to include just `name` named argument 

Patch:
diff --git a/bandit/core/blacklisting.py b/bandit/core/blacklisting.py
index aecf151..81cac56 100644
--- a/bandit/core/blacklisting.py
+++ b/bandit/core/blacklisting.py
@@ -47,7 +47,10 @@ def blacklist(context, config):
             # argument name as an actual import module name.
             # Will produce None if argument is not a literal or identifier
             if name in ["importlib.import_module", "importlib.__import__"]:
-                name = context.call_args[0]
+                if context.call_args_count > 0:
+                    name = context.call_args[0]
+                else:
+                    name = context.call_keywords['name']
         for check in blacklists[node_type]:
             for qn in check['qualnames']:
                 if name is not None and fnmatch.fnmatch(name, qn):
diff --git a/examples/imports-with-importlib.py b/examples/imports-with-importlib.py
index 194432d..748fc66 100644
--- a/examples/imports-with-importlib.py
+++ b/examples/imports-with-importlib.py
@@ -7,3 +7,9 @@ d = importlib.__import__('subprocess')
 # Do not crash when target is an expression
 e = importlib.import_module(MODULE_MAP[key])
 f = importlib.__import__(MODULE_MAP[key])
+
+# Do not crash when target is a named argument
+g = importlib.import_module(name='sys')
+h = importlib.__import__(name='subprocess')
+i = importlib.import_module(name='subprocess', package='bar.baz')
+j = importlib.__import__(name='sys', package='bar.baz')


Test patch:
diff --git a/tests/functional/test_functional.py b/tests/functional/test_functional.py
index 2d17e4e..df6e219 100644
--- a/tests/functional/test_functional.py
+++ b/tests/functional/test_functional.py
@@ -230,8 +230,8 @@ class FunctionalTests(testtools.TestCase):
     def test_imports_using_importlib(self):
         '''Test for dangerous imports using importlib.'''
         expect = {
-            'SEVERITY': {'UNDEFINED': 0, 'LOW': 2, 'MEDIUM': 0, 'HIGH': 0},
-            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 2}
+            'SEVERITY': {'UNDEFINED': 0, 'LOW': 4, 'MEDIUM': 0, 'HIGH': 0},
+            'CONFIDENCE': {'UNDEFINED': 0, 'LOW': 0, 'MEDIUM': 0, 'HIGH': 4}
         }
         self.check_example('imports-with-importlib.py', expect)