Skip to content

Commit

Permalink
[device/mellanox] Mitigation for security vulnerability (#11877)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
maipbui authored Oct 6, 2022
1 parent 1ad1e19 commit 648ca07
Show file tree
Hide file tree
Showing 13 changed files with 110 additions and 127 deletions.
4 changes: 2 additions & 2 deletions device/mellanox/x86_64-mlnx_msn2700-r0/plugins/fanutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class FanUtil(FanBase):

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

def _get_sku_name(self):
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
out, err = p.communicate()
return out.rstrip('\n')

Expand Down
4 changes: 2 additions & 2 deletions device/mellanox/x86_64-mlnx_msn2700-r0/plugins/psuutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class PsuUtil(PsuBase):

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

def _get_sku_name(self):
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
out, err = p.communicate()
return out.rstrip('\n')

Expand Down
24 changes: 12 additions & 12 deletions device/mellanox/x86_64-mlnx_msn2700-r0/plugins/sfputil.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
SYSTEM_READY = 'system_become_ready'
SYSTEM_FAIL = 'system_fail'

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

# Ethernet<n> <=> sfp<n+SFP_PORT_NAME_OFFSET>
SFP_PORT_NAME_OFFSET = 0
Expand Down Expand Up @@ -110,7 +110,7 @@ def port_to_eeprom_mapping(self):
raise Exception()

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

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

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

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

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

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

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

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

eeprom_raw = []
ethtool_cmd = "ethtool -m {} hex on offset {} length {}".format(sfpname, offset, num_bytes)
ethtool_cmd = ["ethtool", "-m", sfpname, "hex", "on", "offset", str(offset), "length", str(num_bytes)]
try:
output = subprocess.check_output(ethtool_cmd, shell=True, universal_newlines=True)
output = subprocess.check_output(ethtool_cmd, universal_newlines=True)
output_lines = output.splitlines()
first_line_raw = output_lines[0]
if "Offset" in first_line_raw:
Expand Down
4 changes: 2 additions & 2 deletions device/mellanox/x86_64-mlnx_msn2700-r0/plugins/thermalutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,12 @@ class ThermalUtil(ThermalBase):

MAX_PSU_FAN = 1
MAX_NUM_PSU = 2
GET_HWSKU_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.hwsku"
GET_HWSKU_CMD = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.hwsku"]
number_of_thermals = 0
thermal_list = []

def _get_sku_name(self):
p = subprocess.Popen(self.GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
p = subprocess.Popen(self.GET_HWSKU_CMD, universal_newlines=True, stdout=subprocess.PIPE)
out, err = p.communicate()
return out.rstrip('\n')

Expand Down
Loading

0 comments on commit 648ca07

Please sign in to comment.