Skip to content

Commit 06e1a0b

Browse files
maipbuimaipbui
authored
[device/dell] Mitigation for security vulnerability (#11875)
Dependency: [PR (#12065)](#12065) needs to merge first. #### Why I did it `commands` module is not protected against malicious input `getstatusoutput` is detected without a static string, uses `shell=True` #### How I did it Eliminate the use of `commands` Use `subprocess.run()`, commands in `subprorcess.run()` are totally static Fix indentation #### How to verify it Tested on DUT [dell_log.txt](https://github.com/sonic-net/sonic-buildimage/files/9561332/dell_log.txt)
1 parent ba5c26a commit 06e1a0b

File tree

36 files changed

+722
-683
lines changed

36 files changed

+722
-683
lines changed

‎device/dell/x86_64-dellemc_n3248pxe_c3338-r0/plugins/fanutil.py

+14-15
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
# Platform-specific FAN status interface for SONiC
44
#
55

6-
import commands
76
import sys
7+
from sonic_py_common.general import getstatusoutput_noshell
88

9-
SENSORS_CMD = "docker exec -i pmon /usr/bin/sensors"
9+
SENSORS_CMD = ["docker", "exec", "-i", "pmon", "/usr/bin/sensors"]
1010
DOCKER_SENSORS_CMD = "/usr/bin/sensors"
1111

1212

@@ -33,24 +33,23 @@ def isDockerEnv(self):
3333
return True
3434

3535
def get_num_fans(self):
36-
n3248pxe_MAX_FANTRAYS = 3
37-
return n3248pxe_MAX_FANTRAYS
36+
n3248pxe_MAX_FANTRAYS = 3
37+
return n3248pxe_MAX_FANTRAYS
3838

3939
def get_presence(self, idx):
40-
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
41-
return int(open(sysfs_path).read(), 16)
40+
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
41+
return int(open(sysfs_path).read(), 16)
4242

4343
def get_direction(self, idx):
44-
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_dir"
45-
return open(sysfs_path).read()
44+
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_dir"
45+
return open(sysfs_path).read()
4646

4747
def get_speed(self, idx):
4848
dockerenv = self.isDockerEnv()
4949
if not dockerenv:
50-
status, cmd_output = commands.getstatusoutput(SENSORS_CMD)
51-
else :
52-
status, cmd_output = commands.getstatusoutput(DOCKER_SENSORS_CMD)
53-
50+
status, cmd_output = getstatusoutput_noshell(SENSORS_CMD)
51+
else:
52+
status, cmd_output = getstatusoutput_noshell(DOCKER_SENSORS_CMD)
5453
if status:
5554
print('Failed to execute sensors command')
5655
sys.exit(0)
@@ -64,9 +63,9 @@ def get_speed(self, idx):
6463
return 0.0
6564

6665
def get_status(self, idx):
67-
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
68-
return int(open(sysfs_path).read(), 16)
66+
sysfs_path = "/sys/devices/platform/dell-n3248pxe-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
67+
return int(open(sysfs_path).read(), 16)
6968

7069

7170
def set_speed(self, idx):
72-
return False
71+
return False

‎device/dell/x86_64-dellemc_n3248pxe_c3338-r0/plugins/psuutil.py

+5-6
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
# Platform-specific PSU status interface for SONiC
44
#
55

6-
import commands
76
import os
87
import sys
8+
from sonic_py_common.general import getstatusoutput_noshell
99

10-
SENSORS_CMD = "docker exec -i pmon /usr/bin/sensors"
10+
SENSORS_CMD = ["docker", "exec", "-i", "pmon", "/usr/bin/sensors"]
1111
DOCKER_SENSORS_CMD = "/usr/bin/sensors"
1212

1313
try:
@@ -95,10 +95,9 @@ def get_psu_presence(self, index):
9595
def get_sensor(self):
9696
dockerenv = self.isDockerEnv()
9797
if not dockerenv:
98-
status, cmd_output = commands.getstatusoutput(SENSORS_CMD)
99-
else :
100-
status, cmd_output = commands.getstatusoutput(DOCKER_SENSORS_CMD)
101-
98+
status, cmd_output = getstatusoutput_noshell(SENSORS_CMD)
99+
else:
100+
status, cmd_output = getstatusoutput_noshell(DOCKER_SENSORS_CMD)
102101
if status:
103102
print('Failed to execute sensors command')
104103
sys.exit(0)

‎device/dell/x86_64-dellemc_n3248te_c3338-r0/plugins/fanutil.py

+14-15
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
# Platform-specific FAN status interface for SONiC
44
#
55

6-
import subprocess
76
import sys
7+
from sonic_py_common.general import getstatusoutput_noshell
88

9-
SENSORS_CMD = "docker exec -i pmon /usr/bin/sensors"
9+
SENSORS_CMD = ["docker", "exec", "-i", "pmon", "/usr/bin/sensors"]
1010
DOCKER_SENSORS_CMD = "/usr/bin/sensors"
1111

1212

@@ -33,24 +33,23 @@ def isDockerEnv(self):
3333
return True
3434

3535
def get_num_fans(self):
36-
N3248TE_MAX_FANTRAYS = 3
37-
return N3248TE_MAX_FANTRAYS
36+
N3248TE_MAX_FANTRAYS = 3
37+
return N3248TE_MAX_FANTRAYS
3838

3939
def get_presence(self, idx):
40-
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
41-
return int(open(sysfs_path).read(), 16)
40+
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
41+
return int(open(sysfs_path).read(), 16)
4242

4343
def get_direction(self, idx):
44-
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_dir"
45-
return open(sysfs_path).read()
44+
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_dir"
45+
return open(sysfs_path).read()
4646

4747
def get_speed(self, idx):
4848
dockerenv = self.isDockerEnv()
4949
if not dockerenv:
50-
status, cmd_output = subprocess.getstatusoutput(SENSORS_CMD)
51-
else :
52-
status, cmd_output = subprocess.getstatusoutput(DOCKER_SENSORS_CMD)
53-
50+
status, cmd_output = getstatusoutput_noshell(SENSORS_CMD)
51+
else:
52+
status, cmd_output = getstatusoutput_noshell(DOCKER_SENSORS_CMD)
5453
if status:
5554
print('Failed to execute sensors command')
5655
sys.exit(0)
@@ -64,9 +63,9 @@ def get_speed(self, idx):
6463
return 0.0
6564

6665
def get_status(self, idx):
67-
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
68-
return int(open(sysfs_path).read(), 16)
66+
sysfs_path = "/sys/devices/platform/dell-n3248te-cpld.0/fan" + self._fan_mapping[idx] + "_prs"
67+
return int(open(sysfs_path).read(), 16)
6968

7069

7170
def set_speed(self, idx):
72-
return False
71+
return False

‎device/dell/x86_64-dellemc_n3248te_c3338-r0/plugins/psuutil.py

+5-6
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
# Platform-specific PSU status interface for SONiC
44
#
55

6-
import commands
76
import os
87
import sys
8+
from sonic_py_common.general import getstatusoutput_noshell
99

10-
SENSORS_CMD = "docker exec -i pmon /usr/bin/sensors"
10+
SENSORS_CMD = ["docker", "exec", "-i", "pmon", "/usr/bin/sensors"]
1111
DOCKER_SENSORS_CMD = "/usr/bin/sensors"
1212

1313
try:
@@ -95,10 +95,9 @@ def get_psu_presence(self, index):
9595
def get_sensor(self):
9696
dockerenv = self.isDockerEnv()
9797
if not dockerenv:
98-
status, cmd_output = commands.getstatusoutput(SENSORS_CMD)
99-
else :
100-
status, cmd_output = commands.getstatusoutput(DOCKER_SENSORS_CMD)
101-
98+
status, cmd_output = getstatusoutput_noshell(SENSORS_CMD)
99+
else:
100+
status, cmd_output = getstatusoutput_noshell(DOCKER_SENSORS_CMD)
102101
if status:
103102
print('Failed to execute sensors command')
104103
sys.exit(0)

‎device/dell/x86_64-dellemc_s5212f_c3538-r0/plugins/psuutil.py

+7-5
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55

66
import logging
77
import sys
8-
import subprocess
8+
from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe
99

1010
S5212F_MAX_PSUS = 2
11-
IPMI_PSU_DATA = "docker exec -it pmon ipmitool sdr list"
12-
IPMI_PSU_DATA_DOCKER = "ipmitool sdr list"
11+
IPMI_PSU_DATA = ["docker", "exec", "-it", "pmon", "ipmitool", "sdr", "list"]
12+
IPMI_PSU_DATA_DOCKER = ["ipmitool", "sdr", "list"]
1313
PSU_PRESENCE = "PSU{0}_stat"
1414
# Use this for older firmware
1515
# PSU_PRESENCE="PSU{0}_prsnt"
@@ -44,7 +44,7 @@ def get_pmc_register(self, reg_name):
4444
if dockerenv == True:
4545
ipmi_cmd = IPMI_PSU_DATA_DOCKER
4646

47-
status, ipmi_sdr_list = subprocess.getstatusoutput(ipmi_cmd)
47+
status, ipmi_sdr_list = getstatusoutput_noshell(ipmi_cmd)
4848

4949
if status:
5050
logging.error('Failed to execute:' + ipmi_sdr_list)
@@ -91,6 +91,8 @@ def get_psu_presence(self, index):
9191
:param index: An integer, index of the PSU of which to query status
9292
:return: Boolean, True if PSU is plugged, False if not
9393
"""
94-
cmd_status, psu_status = subprocess.getstatusoutput('ipmitool raw 0x04 0x2d ' + hex(0x30 + index) + " | awk '{print substr($0,9,1)}'")
94+
ipmi_cmd = ["ipmitool", "raw", "0x04", "0x2d", hex(0x30 + index)]
95+
awk_cmd = ['awk', '{print substr($0,9,1)}']
96+
cmd_status, psu_status = getstatusoutput_noshell_pipe(ipmi_cmd, awk_cmd)
9597
return 1 if psu_status == '1' else 0
9698

‎device/dell/x86_64-dellemc_s5224f_c3538-r0/plugins/psuutil.py

+5-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import logging
88
import sys
9-
import subprocess
9+
from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe
1010

1111
S5224F_MAX_PSUS = 2
1212
IPMI_PSU_DATA = "docker exec -it pmon ipmitool sdr list"
@@ -45,7 +45,7 @@ def get_pmc_register(self, reg_name):
4545
if dockerenv == True:
4646
ipmi_cmd = IPMI_PSU_DATA_DOCKER
4747

48-
status, ipmi_sdr_list = subprocess.getstatusoutput(ipmi_cmd)
48+
status, ipmi_sdr_list = getstatusoutput_noshell(ipmi_cmd)
4949

5050
if status:
5151
logging.error('Failed to execute:' + ipmi_sdr_list)
@@ -92,6 +92,8 @@ def get_psu_presence(self, index):
9292
:param index: An integer, index of the PSU of which to query status
9393
:return: Boolean, True if PSU is plugged, False if not
9494
"""
95-
cmd_status, psu_status = subprocess.getstatusoutput('ipmitool raw 0x04 0x2d ' + hex(0x30 + index) + " | awk '{print substr($0,9,1)}'")
95+
ipmi_cmd = ["ipmitool", "raw", "0x04", "0x2d", hex(0x30 + index)]
96+
awk_cmd = ['awk', '{print substr($0,9,1)}']
97+
cmd_status, psu_status = getstatusoutput_noshell_pipe(ipmi_cmd, awk_cmd)
9698
return 1 if psu_status == '1' else 0
9799

‎device/dell/x86_64-dellemc_s5232f_c3538-r0/plugins/psuutil.py

+32-30
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,17 @@
44
#
55

66

7-
import os.path
87
import logging
98
import sys
10-
11-
if sys.version_info[0] < 3:
12-
import commands
13-
else:
14-
import subprocess as commands
15-
9+
from sonic_py_common.general import getstatusoutput_noshell_pipe
1610

1711
S5232F_MAX_PSUS = 2
18-
IPMI_PSU1_DATA = "docker exec -it pmon ipmitool raw 0x04 0x2d 0x31 | awk '{print substr($0,9,1)}'"
19-
IPMI_PSU1_DATA_DOCKER = "ipmitool raw 0x04 0x2d 0x31 | awk '{print substr($0,9,1)}'"
20-
IPMI_PSU2_DATA = "docker exec -it pmon ipmitool raw 0x04 0x2d 0x32 | awk '{print substr($0,9,1)}'"
21-
IPMI_PSU2_DATA_DOCKER = "ipmitool raw 0x04 0x2d 0x32 | awk '{print substr($0,9,1)}'"
12+
IPMI_PSU1_DATA = ["docker", "exec", "-it", "pmon", "ipmitool", "raw", "0x04", "0x2d", "0x31"]
13+
IPMI_PSU1_DATA_DOCKER = ["ipmitool", "raw", "0x04", "0x2d", "0x31"]
14+
IPMI_PSU2_DATA = ["docker", "exec", "-it", "pmon", "ipmitool", "raw", "0x04", "0x2d", "0x32"]
15+
IPMI_PSU2_DATA_DOCKER = ["ipmitool", "raw", "0x04", "0x2d", "0x32"]
2216
PSU_PRESENCE = "PSU{0}_stat"
17+
awk_cmd = ['awk', '{print substr($0,9,1)}']
2318
# Use this for older firmware
2419
# PSU_PRESENCE="PSU{0}_prsnt"
2520

@@ -44,23 +39,24 @@ def isDockerEnv(self):
4439
return False
4540

4641
# Fetch a BMC register
47-
def get_pmc_register(self, reg_name):
42+
def get_pmc_register(self, index):
4843

4944
status = 1
50-
ipmi_cmd_1 = IPMI_PSU1_DATA
51-
ipmi_cmd_2 = IPMI_PSU1_DATA
45+
ipmi_cmd = ''
5246
dockerenv = self.isDockerEnv()
5347
if dockerenv == True:
5448
if index == 1:
55-
status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA_DOCKER)
49+
ipmi_cmd = IPMI_PSU1_DATA_DOCKER
5650
elif index == 2:
57-
status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA_DOCKER)
51+
ipmi_cmd = IPMI_PSU2_DATA_DOCKER
5852
else:
5953
if index == 1:
60-
status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA)
54+
ipmi_cmd = IPMI_PSU1_DATA
6155
elif index == 2:
62-
status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA)
63-
56+
ipmi_cmd = IPMI_PSU2_DATA
57+
if ipmi_cmd != '':
58+
status, ipmi_sdr_list = getstatusoutput_noshell_pipe(ipmi_cmd, awk_cmd)
59+
6460
if status:
6561
logging.error('Failed to execute ipmitool')
6662
sys.exit(0)
@@ -87,22 +83,25 @@ def get_psu_status(self, index):
8783
"""
8884
# Until psu_status is implemented this is hardcoded temporarily
8985

90-
psu_status = 'f'
86+
psu_status = ''
9187
ret_status = 1
88+
ipmi_cmd = ''
9289
dockerenv = self.isDockerEnv()
9390
if dockerenv == True:
9491
if index == 1:
95-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA_DOCKER)
92+
ipmi_cmd = IPMI_PSU1_DATA_DOCKER
9693
elif index == 2:
97-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA_DOCKER)
94+
ipmi_cmd = IPMI_PSU2_DATA_DOCKER
9895
else:
9996
if index == 1:
100-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA)
97+
ipmi_cmd = IPMI_PSU1_DATA
10198
elif index == 2:
102-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA)
99+
ipmi_cmd = IPMI_PSU2_DATA
100+
if ipmi_cmd != '':
101+
ret_status, ipmi_sdr_list = getstatusoutput_noshell_pipe(ipmi_cmd, awk_cmd)
103102

104103
if ret_status:
105-
logging.error('Failed to execute ipmitool : ')
104+
logging.error('Failed to execute ipmitool')
106105
sys.exit(0)
107106

108107
psu_status = ipmi_sdr_list
@@ -117,20 +116,23 @@ def get_psu_presence(self, index):
117116
"""
118117
psu_status = '0'
119118
ret_status = 1
119+
ipmi_cmd = ''
120120
dockerenv = self.isDockerEnv()
121121
if dockerenv == True:
122122
if index == 1:
123-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA_DOCKER)
123+
ipmi_cmd = IPMI_PSU1_DATA_DOCKER
124124
elif index == 2:
125-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA_DOCKER)
125+
ipmi_cmd = IPMI_PSU2_DATA_DOCKER
126126
else:
127127
if index == 1:
128-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU1_DATA)
128+
ipmi_cmd = IPMI_PSU1_DATA
129129
elif index == 2:
130-
ret_status, ipmi_sdr_list = commands.getstatusoutput(IPMI_PSU2_DATA)
130+
ipmi_cmd = IPMI_PSU2_DATA
131+
if ipmi_cmd != '':
132+
ret_status, ipmi_sdr_list = getstatusoutput_noshell_pipe(ipmi_cmd, awk_cmd)
131133

132134
if ret_status:
133-
logging.error('Failed to execute ipmitool : ')
135+
logging.error('Failed to execute ipmitool')
134136
sys.exit(0)
135137

136138
psu_status = ipmi_sdr_list

‎device/dell/x86_64-dellemc_s5248f_c3538-r0/plugins/psuutil.py

+4-9
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,14 @@
44
#
55

66

7-
import os.path
87
import logging
98
import sys
10-
11-
if sys.version_info[0] < 3:
12-
import commands
13-
else:
14-
import subprocess as commands
9+
from sonic_py_common.general import getstatusoutput_noshell
1510

1611

1712
S5248F_MAX_PSUS = 2
18-
IPMI_PSU_DATA = "docker exec -it pmon ipmitool sdr list"
19-
IPMI_PSU_DATA_DOCKER = "ipmitool sdr list"
13+
IPMI_PSU_DATA = ["docker", "exec", "-it", "pmon", "ipmitool", "sdr", "list"]
14+
IPMI_PSU_DATA_DOCKER = ["ipmitool", "sdr", "list"]
2015
PSU_PRESENCE = "PSU{0}_stat"
2116
# Use this for older firmware
2217
# PSU_PRESENCE="PSU{0}_prsnt"
@@ -53,7 +48,7 @@ def get_pmc_register(self, reg_name):
5348
if dockerenv == True:
5449
ipmi_cmd = IPMI_PSU_DATA_DOCKER
5550

56-
status, ipmi_sdr_list = commands.getstatusoutput(ipmi_cmd)
51+
status, ipmi_sdr_list = getstatusoutput_noshell(ipmi_cmd)
5752

5853
if status:
5954
logging.error('Failed to execute:' + ipmi_sdr_list)

0 commit comments

Comments
 (0)