Skip to content

Commit 648ca07

Browse files
authored
[device/mellanox] Mitigation for security vulnerability (#11877)
Signed-off-by: maipbui <[email protected]> Dependency: [PR (#12065)](#12065) needs to merge first. #### Why I did it `subprocess.Popen()` and `subprocess.check_output()` is used with `shell=True`, which is very dangerous for shell injection. #### How I did it Disable `shell=True`, enable `shell=False` #### How to verify it Tested on DUT, compare and verify the output between the original behavior and the new changes' behavior. [testresults.zip](https://github.com/sonic-net/sonic-buildimage/files/9550867/testresults.zip)
1 parent 1ad1e19 commit 648ca07

File tree

13 files changed

+110
-127
lines changed

13 files changed

+110
-127
lines changed

device/mellanox/x86_64-mlnx_msn2700-r0/plugins/fanutil.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class FanUtil(FanBase):
4444

4545
PWM_MAX = 255
4646
MAX_FAN_PER_DRAWER = 2
47-
GET_HWSKU_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.hwsku"
47+
GET_HWSKU_CMD = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.hwsku"]
4848
sku_without_fan_direction = ['ACS-MSN2010', 'ACS-MSN2100', 'ACS-MSN2410',
4949
'ACS-MSN2700', 'Mellanox-SN2700', 'Mellanox-SN2700-D48C8', 'LS-SN2700', 'ACS-MSN2740']
5050
sku_with_unpluggable_fan = ['ACS-MSN2010', 'ACS-MSN2100']
@@ -72,7 +72,7 @@ def __init__(self):
7272
self.num_of_fan, self.num_of_drawer = self._extract_num_of_fans_and_fan_drawers()
7373

7474
def _get_sku_name(self):
75-
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
75+
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
7676
out, err = p.communicate()
7777
return out.rstrip('\n')
7878

device/mellanox/x86_64-mlnx_msn2700-r0/plugins/psuutil.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class PsuUtil(PsuBase):
4242

4343
MAX_PSU_FAN = 1
4444
MAX_NUM_PSU = 2
45-
GET_HWSKU_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.hwsku"
45+
GET_HWSKU_CMD = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.hwsku"]
4646
# for spectrum1 switches with plugable PSUs, the output voltage file is psuX_volt
4747
# for spectrum2 switches the output voltage file is psuX_volt_out2
4848
sku_spectrum1_with_plugable_psu = ['ACS-MSN2410', 'ACS-MSN2700',
@@ -65,7 +65,7 @@ def __init__(self):
6565
self.fan_speed = "thermal/psu{}_fan1_speed_get"
6666

6767
def _get_sku_name(self):
68-
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
68+
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
6969
out, err = p.communicate()
7070
return out.rstrip('\n')
7171

device/mellanox/x86_64-mlnx_msn2700-r0/plugins/sfputil.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
SYSTEM_READY = 'system_become_ready'
4949
SYSTEM_FAIL = 'system_fail'
5050

51-
GET_PLATFORM_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.platform"
51+
GET_PLATFORM_CMD = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.platform"]
5252

5353
# Ethernet<n> <=> sfp<n+SFP_PORT_NAME_OFFSET>
5454
SFP_PORT_NAME_OFFSET = 0
@@ -110,7 +110,7 @@ def port_to_eeprom_mapping(self):
110110
raise Exception()
111111

112112
def get_port_position_tuple_by_platform_name(self):
113-
p = subprocess.Popen(GET_PLATFORM_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
113+
p = subprocess.Popen(GET_PLATFORM_CMD, universal_newlines=True, stdout=subprocess.PIPE)
114114
out, err = p.communicate()
115115
position_tuple = port_position_tuple_list[platform_dict[out.rstrip('\n')]]
116116
return position_tuple
@@ -136,9 +136,9 @@ def get_presence(self, port_num):
136136
port_num += SFP_PORT_NAME_OFFSET
137137
sfpname = SFP_PORT_NAME_CONVENTION.format(port_num)
138138

139-
ethtool_cmd = "ethtool -m {} 2>/dev/null".format(sfpname)
139+
ethtool_cmd = ["ethtool", "-m", sfpname]
140140
try:
141-
proc = subprocess.Popen(ethtool_cmd, stdout=subprocess.PIPE, shell=True, universal_newlines=True, stderr=subprocess.STDOUT)
141+
proc = subprocess.Popen(ethtool_cmd, stdout=subprocess.PIPE, universal_newlines=True, stderr=subprocess.DEVNULL)
142142
stdout = proc.communicate()[0]
143143
proc.wait()
144144
result = stdout.rstrip('\n')
@@ -155,10 +155,10 @@ def get_low_power_mode(self, port_num):
155155
if port_num < self.port_start or port_num > self.port_end:
156156
return False
157157

158-
lpm_cmd = "docker exec syncd python /usr/share/sonic/platform/plugins/sfplpmget.py {}".format(port_num)
158+
lpm_cmd = ["docker", "exec", "syncd", "python", "/usr/share/sonic/platform/plugins/sfplpmget.py", str(port_num)]
159159

160160
try:
161-
output = subprocess.check_output(lpm_cmd, shell=True, universal_newlines=True)
161+
output = subprocess.check_output(lpm_cmd, universal_newlines=True)
162162
if 'LPM ON' in output:
163163
return True
164164
except subprocess.CalledProcessError as e:
@@ -178,11 +178,11 @@ def set_low_power_mode(self, port_num, lpmode):
178178

179179
# Compose LPM command
180180
lpm = 'on' if lpmode else 'off'
181-
lpm_cmd = "docker exec syncd python /usr/share/sonic/platform/plugins/sfplpmset.py {} {}".format(port_num, lpm)
181+
lpm_cmd = ["docker", "exec", "syncd", "python", "/usr/share/sonic/platform/plugins/sfplpmset.py", str(port_num), lpm]
182182

183183
# Set LPM
184184
try:
185-
subprocess.check_output(lpm_cmd, shell=True, universal_newlines=True)
185+
subprocess.check_output(lpm_cmd, universal_newlines=True)
186186
except subprocess.CalledProcessError as e:
187187
print("Error! Unable to set LPM for {}, rc = {}, err msg: {}".format(port_num, e.returncode, e.output))
188188
return False
@@ -194,10 +194,10 @@ def reset(self, port_num):
194194
if port_num < self.port_start or port_num > self.port_end:
195195
return False
196196

197-
lpm_cmd = "docker exec syncd python /usr/share/sonic/platform/plugins/sfpreset.py {}".format(port_num)
197+
lpm_cmd = ["docker", "exec", "syncd", "python", "/usr/share/sonic/platform/plugins/sfpreset.py", str(port_num)]
198198

199199
try:
200-
subprocess.check_output(lpm_cmd, shell=True, universal_newlines=True)
200+
subprocess.check_output(lpm_cmd, universal_newlines=True)
201201
return True
202202
except subprocess.CalledProcessError as e:
203203
print("Error! Unable to set LPM for {}, rc = {}, err msg: {}".format(port_num, e.returncode, e.output))
@@ -267,9 +267,9 @@ def _read_eeprom_specific_bytes_via_ethtool(self, port_num, offset, num_bytes):
267267
sfpname = SFP_PORT_NAME_CONVENTION.format(port_num)
268268

269269
eeprom_raw = []
270-
ethtool_cmd = "ethtool -m {} hex on offset {} length {}".format(sfpname, offset, num_bytes)
270+
ethtool_cmd = ["ethtool", "-m", sfpname, "hex", "on", "offset", str(offset), "length", str(num_bytes)]
271271
try:
272-
output = subprocess.check_output(ethtool_cmd, shell=True, universal_newlines=True)
272+
output = subprocess.check_output(ethtool_cmd, universal_newlines=True)
273273
output_lines = output.splitlines()
274274
first_line_raw = output_lines[0]
275275
if "Offset" in first_line_raw:

device/mellanox/x86_64-mlnx_msn2700-r0/plugins/thermalutil.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,12 +375,12 @@ class ThermalUtil(ThermalBase):
375375

376376
MAX_PSU_FAN = 1
377377
MAX_NUM_PSU = 2
378-
GET_HWSKU_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.hwsku"
378+
GET_HWSKU_CMD = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.hwsku"]
379379
number_of_thermals = 0
380380
thermal_list = []
381381

382382
def _get_sku_name(self):
383-
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
383+
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
384384
out, err = p.communicate()
385385
return out.rstrip('\n')
386386

0 commit comments

Comments
 (0)