summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Yang <yangyang1@zte.com.cn>2018-01-04 16:40:02 +0800
committerAlex Yang <yangyang1@zte.com.cn>2018-01-04 17:21:38 +0800
commit2334b3c550c217308efbaf3f4f22718c3b3d0466 (patch)
treef901417c8a976103d91749501b737b271f1d6533
parent11f285bb542624446bfcf033532d6ca286cc2cb5 (diff)
Fix security risks about shell=True
Change-Id: I2db012e2b6a4325c42d5422901dea52a5ab7f664 Signed-off-by: Alex Yang <yangyang1@zte.com.cn>
-rw-r--r--deploy/utils.py5
-rw-r--r--tests/unit/test_utils.py19
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', [