From 2334b3c550c217308efbaf3f4f22718c3b3d0466 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Thu, 4 Jan 2018 16:40:02 +0800 Subject: Fix security risks about shell=True Change-Id: I2db012e2b6a4325c42d5422901dea52a5ab7f664 Signed-off-by: Alex Yang --- deploy/utils.py | 5 ++--- tests/unit/test_utils.py | 19 ++++++++----------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/deploy/utils.py b/deploy/utils.py index 55fbc53a..d0e67359 100644 --- a/deploy/utils.py +++ b/deploy/utils.py @@ -124,10 +124,9 @@ def ipmi_reboot_node(host, user, passwd, boot_source=None): def run_shell(cmd, check=False): - process = subprocess.Popen(cmd, + process = subprocess.Popen(cmd.split(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - shell=True) + stderr=subprocess.PIPE) while process.poll() is None: LD(process.stdout.readline().strip()) diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index e3b9dff7..4998a447 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -183,19 +183,16 @@ def test_ipmi_reboot_node(mock_getstatusoutput, mock_err_exit, @pytest.mark.parametrize('cmd, check, expect', [ - ('cd /home', False, 0), - ('cd /home', True, 0), + ('ls /home', False, 0), + ('ls /home', True, 0), ('test_command', False, 127), ('test_command', True, 127)]) -@mock.patch('deploy.utils.err_exit') -def test_run_shell(mock_err_exit, cmd, check, expect): - ret = run_shell(cmd, check=check) - if check: - if cmd == 'cd /home': - mock_err_exit.assert_not_called() - elif cmd == 'test_command': - mock_err_exit.assert_called_once() - assert ret == expect +def test_run_shell(cmd, check, expect): + try: + ret = run_shell(cmd, check=check) + assert ret == expect + except OSError: + assert cmd == 'test_command' @pytest.mark.parametrize('scenario', [ -- cgit 1.2.3-korg