diff --git a/bandit/plugins/injection_shell.py b/bandit/plugins/injection_shell.py index 3c678ee1c..d627cd24f 100644 --- a/bandit/plugins/injection_shell.py +++ b/bandit/plugins/injection_shell.py @@ -15,9 +15,21 @@ def _evaluate_shell_call(context): - no_formatting = isinstance( - context.node.args[0], ast.Constant - ) and isinstance(context.node.args[0].value, str) + command_node = None + if context.node.args: + command_node = context.node.args[0] + else: + for keyword in context.node.keywords: + if keyword.arg in ["args", "cmd", "command"]: + command_node = keyword.value + break + + if command_node is None: + return bandit.HIGH + + no_formatting = isinstance(command_node, ast.Constant) and isinstance( + command_node.value, str + ) if no_formatting: return bandit.LOW diff --git a/tests/unit/test_injection_shell.py b/tests/unit/test_injection_shell.py new file mode 100644 index 000000000..16d190a8e --- /dev/null +++ b/tests/unit/test_injection_shell.py @@ -0,0 +1,146 @@ +import ast +import unittest +from unittest import mock + +import bandit +from bandit.core import issue +from bandit.plugins import injection_shell + + +class InjectionShellTests(unittest.TestCase): + + def _get_context(self, function_name, args=None, keywords=None): + context = mock.Mock() + context.call_function_name_qual = function_name + # context.call_args expects a list of arguments passed to the function call + context.call_args = args or [] + context.call_keywords = keywords or {} + context.node = mock.Mock() + context.node.args = [] + context.node.keywords = [] + # Helpers for getting line number + context.get_lineno_for_call_arg.return_value = 1 + return context + + def test_subprocess_popen_with_shell_true_low_severity(self): + # Case: subprocess.Popen('ls', shell=True) -> LOW severity (static string) + context = self._get_context( + "subprocess.Popen", args=[mock.Mock()], keywords={"shell": True} + ) + + # Mocking context.node for _evaluate_shell_call + # It checks if args[0] is ast.Constant and value is str + arg_node = ast.Constant(value="ls") + context.node.args = [arg_node] + + config = {"subprocess": ["subprocess.Popen"]} + + with mock.patch( + "bandit.plugins.injection_shell.has_shell", return_value=True + ): + result = injection_shell.subprocess_popen_with_shell_equals_true( + context, config + ) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.LOW, result.severity) + self.assertEqual(bandit.HIGH, result.confidence) + self.assertEqual( + "call with shell=True", "call with shell=True" + ) # Dummy assertion + + def test_subprocess_popen_with_shell_true_high_severity(self): + # Case: subprocess.Popen(cmd, shell=True) -> HIGH severity (dynamic) + context = self._get_context( + "subprocess.Popen", args=[mock.Mock()], keywords={"shell": True} + ) + + # Mocking context.node for _evaluate_shell_call + # It checks if args[0] is ast.Constant. If not (e.g. ast.Name), it returns HIGH. + arg_node = ast.Name(id="cmd", ctx=ast.Load()) + context.node.args = [arg_node] + + config = {"subprocess": ["subprocess.Popen"]} + + with mock.patch( + "bandit.plugins.injection_shell.has_shell", return_value=True + ): + result = injection_shell.subprocess_popen_with_shell_equals_true( + context, config + ) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.HIGH, result.severity) + self.assertEqual(bandit.HIGH, result.confidence) + + def test_subprocess_without_shell_equals_true(self): + # Case: subprocess.Popen(['ls'], shell=False) -> LOW severity + context = self._get_context("subprocess.Popen", args=[mock.Mock()]) + config = {"subprocess": ["subprocess.Popen"]} + + with mock.patch( + "bandit.plugins.injection_shell.has_shell", return_value=False + ): + result = injection_shell.subprocess_without_shell_equals_true( + context, config + ) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.LOW, result.severity) + self.assertEqual(bandit.HIGH, result.confidence) + self.assertIn( + "subprocess call - check for execution of untrusted input", + result.text, + ) + + def test_any_other_function_with_shell_equals_true(self): + # Case: custom_function(cmd, shell=True) -> MEDIUM severity + context = self._get_context( + "custom_function", args=[mock.Mock()], keywords={"shell": True} + ) + config = {"subprocess": ["subprocess.Popen"]} + + with mock.patch( + "bandit.plugins.injection_shell.has_shell", return_value=True + ): + result = injection_shell.any_other_function_with_shell_equals_true( + context, config + ) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.MEDIUM, result.severity) + self.assertEqual(bandit.LOW, result.confidence) + + def test_start_process_with_a_shell(self): + # Case: os.system('ls') -> LOW or HIGH depending on input + # LOW if static + context = self._get_context("os.system", args=[mock.Mock()]) + arg_node = ast.Constant(value="ls") + context.node.args = [arg_node] + + config = {"shell": ["os.system"]} + + result = injection_shell.start_process_with_a_shell(context, config) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.LOW, result.severity) + + # HIGH if dynamic + context_dynamic = self._get_context("os.system", args=[mock.Mock()]) + arg_node_dynamic = ast.Name(id="cmd", ctx=ast.Load()) + context_dynamic.node.args = [arg_node_dynamic] + + result_dynamic = injection_shell.start_process_with_a_shell( + context_dynamic, config + ) + self.assertEqual(bandit.HIGH, result_dynamic.severity) + + def test_start_process_with_no_shell(self): + # Case: os.execl(...) -> LOW severity + context = self._get_context("os.execl") + config = {"no_shell": ["os.execl"]} + + result = injection_shell.start_process_with_no_shell(context, config) + + self.assertIsInstance(result, bandit.Issue) + self.assertEqual(bandit.LOW, result.severity)