Skip to content

Commit

Permalink
[nokia] Replace os.system and remove subprocess with shell=True (#12100)
Browse files Browse the repository at this point in the history
Signed-off-by: maipbui <maibui@microsoft.com>
Dependency: [https://github.com/sonic-net/sonic-buildimage/pull/12065](https://github.com/sonic-net/sonic-buildimage/pull/12065)
#### Why I did it
`subprocess.Popen()` and `subprocess.run()` is used with `shell=True`, which is very dangerous for shell injection.
`os` - not secure against maliciously constructed input and dangerous if used to evaluate dynamic content
`getstatusoutput` is dangerous because it contains `shell=True` in the implementation
#### How I did it
Replace `os` by `subprocess`, use with `shell=False`
Remove unused functions
  • Loading branch information
maipbui authored Dec 1, 2022
1 parent ec809bd commit 2b3e884
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 64 deletions.
4 changes: 3 additions & 1 deletion device/nokia/armhf-nokia_ixs7215_52x-r0/plugins/eeprom.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ class board(eeprom_tlvinfo.TlvInfoDecoder):
def __init__(self, name, path, cpld_root, ro):
self.eeprom_path = "/sys/class/i2c-adapter/i2c-0/0-0053/eeprom"
if not os.path.exists(self.eeprom_path):
os.system("echo 24c02 0x53 > /sys/class/i2c-adapter/i2c-0/new_device")
file = "/sys/class/i2c-adapter/i2c-0/new_device"
with open(file, 'w') as f:
f.write('24c02 0x53\n')
super(board, self).__init__(self.eeprom_path, 0, '', True)
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from sonic_platform.thermal import Thermal
from sonic_platform.component import Component
from sonic_py_common import logger
from sonic_py_common.general import getstatusoutput_noshell
except ImportError as e:
raise ImportError(str(e) + "- required module not found")

Expand All @@ -27,11 +28,6 @@
except ImportError as e:
smbus_present = 0

if sys.version_info[0] < 3:
import commands as cmd
else:
import subprocess as cmd

MAX_SELECT_DELAY = 3600
COPPER_PORT_START = 1
COPPER_PORT_END = 48
Expand Down Expand Up @@ -209,7 +205,7 @@ def get_revision(self):
string: Revision value of chassis
"""
if smbus_present == 0: # called from host
cmdstatus, value = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x0')
cmdstatus, value = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x0'])
else:
bus = smbus.SMBus(0)
DEVICE_ADDRESS = 0x41
Expand Down Expand Up @@ -331,7 +327,7 @@ def set_status_led(self, color):

# Write sys led
if smbus_present == 0: # called from host (e.g. 'show system-health')
cmdstatus, value = cmd.getstatusoutput('sudo i2cset -y 0 0x41 0x7 %d' % value)
cmdstatus, value = getstatusoutput_noshell(['sudo', 'i2cset', '-y', '0', '0x41', '0x7', str(value)])
if cmdstatus:
sonic_logger.log_warning(" System LED set %s failed" % value)
return False
Expand All @@ -353,7 +349,7 @@ def get_status_led(self):
"""
# Read sys led
if smbus_present == 0: # called from host
cmdstatus, value = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x7')
cmdstatus, value = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x7'])
value = int(value, 16)
else:
bus = smbus.SMBus(0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@

try:
import os
import sys
import subprocess
import ntpath
from sonic_platform_base.component_base import ComponentBase
from sonic_py_common.general import getstatusoutput_noshell, getstatusoutput_noshell_pipe
except ImportError as e:
raise ImportError(str(e) + "- required module not found")

Expand All @@ -22,11 +22,6 @@
except ImportError as e:
smbus_present = 0

if sys.version_info[0] < 3:
import commands as cmd
else:
import subprocess as cmd


class Component(ComponentBase):
"""Nokia platform-specific Component class"""
Expand All @@ -35,29 +30,20 @@ class Component(ComponentBase):
["System-CPLD", "Used for managing SFPs, LEDs, PSUs and FANs "],
["U-Boot", "Performs initialization during booting"],
]
CPLD_UPDATE_COMMAND = 'cp /usr/sbin/vme /tmp; cp {} /tmp; cd /tmp; ./vme {};'
CPLD_UPDATE_COMMAND1 = ['cp', '/usr/sbin/vme', '/tmp']
CPLD_UPDATE_COMMAND2 = ['cp', '', '/tmp']
CPLD_UPDATE_COMMAND3 = ['cd', '/tmp']
CPLD_UPDATE_COMMAND4 = ['./vme', '']

def __init__(self, component_index):
self.index = component_index
self.name = self.CHASSIS_COMPONENTS[self.index][0]
self.description = self.CHASSIS_COMPONENTS[self.index][1]

def _get_command_result(self, cmdline):
try:
proc = subprocess.Popen(cmdline.split(), stdout=subprocess.PIPE,
stderr=subprocess.STDOUT)
stdout = proc.communicate()[0]
proc.wait()
result = stdout.rstrip('\n')
except OSError:
result = None

return result

def _get_cpld_version(self, cpld_number):

if smbus_present == 0:
cmdstatus, cpld_version = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x2')
cmdstatus, cpld_version = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x2'])
else:
bus = smbus.SMBus(0)
DEVICE_ADDRESS = 0x41
Expand Down Expand Up @@ -144,7 +130,10 @@ def get_firmware_version(self):
return self._get_cpld_version(self.index)

if self.index == 1:
cmdstatus, uboot_version = cmd.getstatusoutput('grep --null-data U-Boot /dev/mtd0ro|head -1 | cut -d" " -f2-4')
cmd1 = ['grep', '--null-data', 'U-Boot', '/dev/mtd0ro']
cmd2 = ['head', '-1']
cmd3 = ['cut', '-d', ' ', '-f2-4']
cmdstatus, uboot_version = getstatusoutput_noshell_pipe(cmd1, cmd2, cmd3)
return uboot_version

def install_firmware(self, image_path):
Expand All @@ -165,12 +154,16 @@ def install_firmware(self, image_path):
print("ERROR: the cpld image {} doesn't exist ".format(image_path))
return False

cmdline = self.CPLD_UPDATE_COMMAND.format(image_path, image_name)
self.CPLD_UPDATE_COMMAND2[1] = image_path
self.CPLD_UPDATE_COMMAND4[1] = image_name

success_flag = False

try:
subprocess.check_call(cmdline, stderr=subprocess.STDOUT, shell=True)

try:
subprocess.check_call(self.CPLD_UPDATE_COMMAND1, stderr=subprocess.STDOUT)
subprocess.check_call(self.CPLD_UPDATE_COMMAND2, stderr=subprocess.STDOUT)
subprocess.check_call(self.CPLD_UPDATE_COMMAND3, stderr=subprocess.STDOUT)
subprocess.check_call(self.CPLD_UPDATE_COMMAND4, stderr=subprocess.STDOUT)
success_flag = True
except subprocess.CalledProcessError as e:
print("ERROR: Failed to upgrade CPLD: rc={}".format(e.returncode))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,13 @@

try:
import os
import sys
from sonic_platform_base.psu_base import PsuBase
from sonic_py_common import logger
from sonic_platform.eeprom import Eeprom
from sonic_py_common.general import getstatusoutput_noshell
except ImportError as e:
raise ImportError(str(e) + "- required module not found")

if sys.version_info[0] < 3:
import commands as cmd
else:
import subprocess as cmd

smbus_present = 1
try:
import smbus
Expand Down Expand Up @@ -86,7 +81,7 @@ def get_presence(self):
"""

if smbus_present == 0: # if called from psuutil outside of pmon
cmdstatus, psustatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0xa')
cmdstatus, psustatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0xa'])
psustatus = int(psustatus, 16)
else:
bus = smbus.SMBus(0)
Expand Down Expand Up @@ -150,7 +145,7 @@ def get_status(self):
"""

if smbus_present == 0:
cmdstatus, psustatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0xa')
cmdstatus, psustatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0xa'])
psustatus = int(psustatus, 16)
sonic_logger.log_warning("PMON psu-smbus - presence = 0 ")
else:
Expand Down Expand Up @@ -179,7 +174,7 @@ def get_voltage(self):
e.g. 12.1
"""
if smbus_present == 0:
cmdstatus, psustatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0xa')
cmdstatus, psustatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0xa'])
psustatus = int(psustatus, 16)
else:
bus = smbus.SMBus(0)
Expand Down Expand Up @@ -226,7 +221,7 @@ def get_powergood_status(self):
"""

if smbus_present == 0:
cmdstatus, psustatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0xa')
cmdstatus, psustatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0xa'])
psustatus = int(psustatus, 16)
else:
bus = smbus.SMBus(0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,18 @@
#
#############################################################################

import os
import sys
import subprocess

try:
from sonic_platform_base.sfp_base import SfpBase
from sonic_platform_base.sonic_sfp.sff8472 import sff8472InterfaceId
from sonic_platform_base.sonic_sfp.sff8472 import sff8472Dom
from sonic_platform_base.sonic_sfp.sfputilhelper import SfpUtilHelper
from sonic_py_common import logger
from sonic_py_common.general import getstatusoutput_noshell
except ImportError as e:
raise ImportError(str(e) + "- required module not found")

if sys.version_info[0] < 3:
import commands as cmd
else:
import subprocess as cmd

smbus_present = 1

try:
Expand Down Expand Up @@ -118,7 +113,7 @@ class Sfp(SfpBase):
# Paths
PLATFORM_ROOT_PATH = "/usr/share/sonic/device"
PMON_HWSKU_PATH = "/usr/share/sonic/hwsku"
HOST_CHK_CMD = "docker > /dev/null 2>&1"
HOST_CHK_CMD = ["docker"]

PLATFORM = "armhf-nokia_ixs7215_52x-r0"
HWSKU = "Nokia-7215"
Expand Down Expand Up @@ -186,7 +181,7 @@ def __convert_string_to_num(self, value_str):
return 'N/A'

def __is_host(self):
return os.system(self.HOST_CHK_CMD) == 0
return subprocess.call(self.HOST_CHK_CMD) == 0

def __get_path_to_port_config_file(self):
platform_path = "/".join([self.PLATFORM_ROOT_PATH, self.PLATFORM])
Expand Down Expand Up @@ -811,7 +806,7 @@ def tx_disable(self, tx_disable):
return False

if smbus_present == 0: # if called from sfputil outside of pmon
cmdstatus, register = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x5')
cmdstatus, register = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x5'])
if cmdstatus:
sonic_logger.log_warning("sfp cmdstatus i2c get failed %s" % register )
return False
Expand All @@ -824,13 +819,13 @@ def tx_disable(self, tx_disable):

pos = [1, 2, 4, 8]
mask = pos[self.index-SFP_PORT_START]
if tx_disable == True:
if tx_disable is True:
setbits = register | mask
else:
setbits = register & ~mask

if smbus_present == 0: # if called from sfputil outside of pmon
cmdstatus, output = cmd.getstatusoutput('sudo i2cset -y -m 0x0f 0 0x41 0x5 %d' % setbits)
cmdstatus, output = getstatusoutput_noshell(['sudo', 'i2cset', '-y', '-m', '0x0f', '0', '0x41', '0x5', str(setbits)])
if cmdstatus:
sonic_logger.log_warning("sfp cmdstatus i2c write failed %s" % output )
return False
Expand Down Expand Up @@ -912,7 +907,7 @@ def get_presence(self):
return False

if smbus_present == 0: # if called from sfputil outside of pmon
cmdstatus, sfpstatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x3')
cmdstatus, sfpstatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x3'])
sfpstatus = int(sfpstatus, 16)
else:
bus = smbus.SMBus(0)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
'''
listen for the SFP change event and return to chassis.
'''
import sys
import time
from sonic_py_common import logger
from sonic_py_common.general import getstatusoutput_noshell

smbus_present = 1

Expand All @@ -12,11 +12,6 @@
except ImportError as e:
smbus_present = 0

if sys.version_info[0] < 3:
import commands as cmd
else:
import subprocess as cmd

# system level event/error
EVENT_ON_ALL_SFP = '-1'
SYSTEM_NOT_READY = 'system_not_ready'
Expand Down Expand Up @@ -51,7 +46,7 @@ def deinitialize(self):
def _get_transceiver_status(self):
if smbus_present == 0:
sonic_logger.log_info(" PMON - smbus ERROR - DEBUG sfp_event ")
cmdstatus, sfpstatus = cmd.getstatusoutput('sudo i2cget -y 0 0x41 0x3')
cmdstatus, sfpstatus = getstatusoutput_noshell(['sudo', 'i2cget', '-y', '0', '0x41', '0x3'])
sfpstatus = int(sfpstatus, 16)
else:
bus = smbus.SMBus(0)
Expand Down

0 comments on commit 2b3e884

Please sign in to comment.