Skip to content

Commit 5238bd7

Browse files
maipbuiStormLiangMS
authored andcommitted
[ruijie] Replace os.system and remove subprocess with shell=True (#12107)
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 1. `getstatusoutput` is used without a static string and it uses `shell=True` 2. `subprocess()` - when using with `shell=True` is dangerous. Using subprocess function without a static string can lead to command injection. 3. `os` - not secure against maliciously constructed input and dangerous if used to evaluate dynamic content. #### How I did it 1. use `getstatusoutput` without shell=True 2. `subprocess()` - use `shell=False` instead. use an array string. Ref: [https://semgrep.dev/docs/cheat-sheets/python-command-injection/#mitigation](https://semgrep.dev/docs/cheat-sheets/python-command-injection/#mitigation) 3. `os` - use with `subprocess`
1 parent 48d4c0a commit 5238bd7

File tree

8 files changed

+151
-152
lines changed

8 files changed

+151
-152
lines changed

platform/broadcom/sonic-platform-modules-ruijie/b6510-48vs8cq/sonic_pcie/pcie_common.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,14 @@ def get_pcie_device(self):
3737
pciList = []
3838
p1 = "^(\w+):(\w+)\.(\w)\s(.*)\s*\(*.*\)*"
3939
p2 = "^.*:.*:.*:(\w+)\s*\(*.*\)*"
40-
command1 = "sudo lspci"
41-
command2 = "sudo lspci -n"
40+
command1 = ["sudo", "lspci"]
41+
command2 = ["sudo", "lspci", "-n"]
4242
# run command 1
43-
proc1 = subprocess.Popen(command1, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
43+
proc1 = subprocess.Popen(command1, universal_newlines=True, stdout=subprocess.PIPE)
4444
output1 = proc1.stdout.readlines()
4545
proc1.communicate()
4646
# run command 2
47-
proc2 = subprocess.Popen(command2, shell=True, universal_newlines=True, stdout=subprocess.PIPE)
47+
proc2 = subprocess.Popen(command2, universal_newlines=True, stdout=subprocess.PIPE)
4848
output2 = proc2.stdout.readlines()
4949
proc2.communicate()
5050

platform/broadcom/sonic-platform-modules-ruijie/b6510-48vs8cq/sonic_platform/chassis.py

+27-26
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@
1010

1111
try:
1212
import time
13-
import subprocess
1413
from sonic_platform_base.chassis_base import ChassisBase
1514
from sonic_platform.common import Common
1615
from sonic_platform.sfp import Sfp
1716
from sonic_platform.sfp import PORT_START
1817
from sonic_platform.sfp import PORTS_IN_BLOCK
1918
from sonic_platform.logger import logger
19+
from sonic_py_common.general import getstatusoutput_noshell
2020
except ImportError as e:
2121
raise ImportError(str(e) + "- required module not found")
2222

@@ -36,17 +36,17 @@ def __init__(self):
3636
self.SFP_STATUS_INSERTED = "1"
3737
self.SFP_STATUS_REMOVED = "0"
3838
self.port_dict = {}
39-
self.enable_read= "i2cset -f -y 2 0x35 0x2a 0x01"
40-
self.disable_read = "i2cset -f -y 2 0x35 0x2a 0x00"
41-
self.enable_write = "i2cset -f -y 2 0x35 0x2b 0x00"
42-
self.disable_write = "i2cset -f -y 2 0x35 0x2b 0x01"
43-
self.enable_erase = "i2cset -f -y 2 0x35 0x2c 0x01"
44-
self.disable_erase = "i2cset -f -y 2 0x35 0x2c 0x00"
45-
self.read_value = "i2cget -f -y 2 0x35 0x25"
46-
self.write_value = "i2cset -f -y 2 0x35 0x21 0x0a"
47-
self.set_sys_led_cmd = "i2cset -f -y 2 0x33 0xb2 "
48-
self.get_sys_led_cmd = "i2cget -f -y 2 0x33 0xb2"
49-
self.led_status = "red"
39+
self.enable_read= ["i2cset", "-f", "-y", "2", "0x35", "0x2a", "0x01"]
40+
self.disable_read = ["i2cset", "-f", "-y", "2", "0x35", "0x2a", "0x00"]
41+
self.enable_write = ["i2cset", "-f", "-y", "2", "0x35", "0x2b", "0x00"]
42+
self.disable_write = ["i2cset", "-f", "-y", "2", "0x35", "0x2b", "0x01"]
43+
self.enable_erase = ["i2cset", "-f", "-y", "2", "0x35", "0x2c", "0x01"]
44+
self.disable_erase = ["i2cset", "-f", "-y", "2", "0x35", "0x2c", "0x00"]
45+
self.read_value = ["i2cget", "-f", "-y", "2", "0x35", "0x25"]
46+
self.write_value = ["i2cset", "-f", "-y", "2", "0x35", "0x21", "0x0a"]
47+
self.set_sys_led_cmd = ["i2cset", "-f", "-y", "2", "0x33", "0xb2"]
48+
self.get_sys_led_cmd = ["i2cget", "-f", "-y", "2", "0x33", "0xb2"]
49+
self.led_status = "red"
5050
# Initialize SFP list
5151
# sfp.py will read eeprom contents and retrive the eeprom data.
5252
# It will also provide support sfp controls like reset and setting
@@ -210,25 +210,25 @@ def get_reboot_cause(self):
210210
try:
211211
is_power_loss = False
212212
# enable read
213-
subprocess.getstatusoutput(self.disable_write)
214-
subprocess.getstatusoutput(self.enable_read)
215-
ret , log = subprocess.getstatusoutput(self.read_value)
213+
getstatusoutput_noshell(self.disable_write)
214+
getstatusoutput_noshell(self.enable_read)
215+
ret , log = getstatusoutput_noshell(self.read_value)
216216
if ret == 0 and "0x0a" in log:
217217
is_power_loss = True
218218

219219
# erase i2c and e2
220-
subprocess.getstatusoutput(self.enable_erase)
220+
getstatusoutput_noshell(self.enable_erase)
221221
time.sleep(1)
222-
subprocess.getstatusoutput(self.disable_erase)
222+
getstatusoutput_noshell(self.disable_erase)
223223
# clear data
224-
subprocess.getstatusoutput(self.enable_write)
225-
subprocess.getstatusoutput(self.disable_read)
226-
subprocess.getstatusoutput(self.disable_write)
227-
subprocess.getstatusoutput(self.enable_read)
224+
getstatusoutput_noshell(self.enable_write)
225+
getstatusoutput_noshell(self.disable_read)
226+
getstatusoutput_noshell(self.disable_write)
227+
getstatusoutput_noshell(self.enable_read)
228228
# enable write and set data
229-
subprocess.getstatusoutput(self.enable_write)
230-
subprocess.getstatusoutput(self.disable_read)
231-
subprocess.getstatusoutput(self.write_value)
229+
getstatusoutput_noshell(self.enable_write)
230+
getstatusoutput_noshell(self.disable_read)
231+
getstatusoutput_noshell(self.write_value)
232232
if is_power_loss:
233233
return(self.REBOOT_CAUSE_POWER_LOSS, None)
234234
except Exception as e:
@@ -417,7 +417,8 @@ def set_status_led(self, color):
417417
if regval is None:
418418
print("Invaild color input.")
419419
return False
420-
ret , log = subprocess.getstatusoutput(self.set_sys_led_cmd + regval)
420+
cmd = self.set_sys_led_cmd + [regval]
421+
ret, log = getstatusoutput_noshell(cmd)
421422
if ret != 0:
422423
print("Cannot execute %s" % self.set_sys_led_cmd + regval)
423424
return False
@@ -431,7 +432,7 @@ def get_status_led(self):
431432
A string, one of the valid LED color strings which could be vendor
432433
specified.
433434
"""
434-
ret , log = subprocess.getstatusoutput(self.get_sys_led_cmd)
435+
ret , log = getstatusoutput_noshell(self.get_sys_led_cmd)
435436
if ret != 0:
436437
print("Cannot execute %s" % self.get_sys_led_cmd)
437438
return False

platform/broadcom/sonic-platform-modules-ruijie/b6510-48vs8cq/sonic_platform/common.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import os
22
import yaml
3-
3+
import subprocess
44
from sonic_py_common import device_info
55

66

@@ -10,13 +10,13 @@ class Common:
1010
PMON_PLATFORM_PATH = '/usr/share/sonic/platform/'
1111
CONFIG_DIR = 'sonic_platform_config'
1212

13-
HOST_CHK_CMD = "docker > /dev/null 2>&1"
13+
HOST_CHK_CMD = ["docker"]
1414

1515
def __init__(self):
1616
(self.platform, self.hwsku) = device_info.get_platform_and_hwsku()
1717

1818
def is_host(self):
19-
return os.system(self.HOST_CHK_CMD) == 0
19+
return subprocess.call(self.HOST_CHK_CMD) == 0
2020

2121
def load_json_file(self, path):
2222
"""

platform/broadcom/sonic-platform-modules-ruijie/b6510-48vs8cq/sonic_platform/component.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
########################################################################
99

1010
try:
11-
import subprocess
1211
from sonic_platform_base.component_base import ComponentBase
1312
from sonic_platform.regutil import Reg
1413
from sonic_platform.logger import logger
14+
from sonic_py_common.general import getstatusoutput_noshell
1515
except ImportError as e:
1616
raise ImportError(str(e) + "- required module not found")
1717

@@ -70,12 +70,12 @@ def install_firmware(self, image_path):
7070
"""
7171
try:
7272
successtips = "CPLD Upgrade succeeded!"
73-
status, output = subprocess.getstatusoutput("which firmware_upgrade")
73+
status, output = getstatusoutput_noshell(["which", "firmware_upgrade"])
7474
if status or len(output) <= 0:
7575
logger.error("no upgrade tool.")
7676
return False
77-
cmdstr = "%s %s cpld %d cpld"%(output,image_path,self.slot)
78-
ret, log = subprocess.getstatusoutput(cmdstr)
77+
cmdstr = [output, image_path, "cpld", self.slot, "cpld"]
78+
ret, log = getstatusoutput_noshell(cmdstr)
7979
if ret == 0 and successtips in log:
8080
return True
8181
logger.error("upgrade failed. ret:%d, log:\n%s" % (ret, log))

platform/broadcom/sonic-platform-modules-ruijie/common/lib/plat_hal/osutil.py

+11-12
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66
#
77
#######################################################
88

9-
import subprocess
109
import time
1110
import glob
1211
import re
1312
#import chardet
1413
from rjutil.smbus import SMBus
1514
import time
1615
from functools import wraps
16+
from sonic_py_common.general import getstatusoutput_noshell
1717

1818

1919
def retry(maxretry =6, delay = 0.01):
@@ -80,13 +80,13 @@ def rji2csetword_python(bus, addr, reg, value):
8080

8181
@staticmethod
8282
def command(cmdstr):
83-
retcode, output = subprocess.getstatusoutput(cmdstr)
83+
retcode, output = getstatusoutput_noshell(cmdstr)
8484
return retcode, output
8585

8686

8787
@staticmethod
8888
def geti2cword_i2ctool(bus, addr, offset):
89-
command_line = "i2cget -f -y %d 0x%02x 0x%02x wp" % (bus, addr, offset)
89+
command_line = ["i2cget", "-f", "-y", str(bus), "0x%02x"%addr, "0x%02x"%offset, "wp"]
9090
retrytime = 6
9191
ret_t = ""
9292
for i in range(retrytime):
@@ -99,7 +99,7 @@ def geti2cword_i2ctool(bus, addr, offset):
9999

100100
@staticmethod
101101
def seti2cword_i2ctool(bus, addr, offset, val):
102-
command_line = "i2cset -f -y %d 0x%02x 0x%0x 0x%04x wp" % (bus, addr, offset, val)
102+
command_line = ["i2cset", "-f", "-y", str(bus), "0x%02x"%addr, "0x%0x"%offset, "0x%04x"%val, "wp"]
103103
retrytime = 6
104104
ret_t = ""
105105
for i in range(retrytime):
@@ -111,7 +111,7 @@ def seti2cword_i2ctool(bus, addr, offset, val):
111111

112112
@staticmethod
113113
def rji2cget_i2ctool(bus, devno, address):
114-
command_line = "i2cget -f -y %d 0x%02x 0x%02x " % (bus, devno, address)
114+
command_line = ["i2cget", "-f", "-y", str(bus), "0x%02x"%devno, "0x%02x"%address]
115115
retrytime = 6
116116
ret_t = ""
117117
for i in range(retrytime):
@@ -123,8 +123,7 @@ def rji2cget_i2ctool(bus, devno, address):
123123

124124
@staticmethod
125125
def rji2cset_i2ctool(bus, devno, address, byte):
126-
command_line = "i2cset -f -y %d 0x%02x 0x%02x 0x%02x" % (
127-
bus, devno, address, byte)
126+
command_line = ["i2cset", "-f", "-y", str(bus), "0x%02x"%devno, "0x%02x"%address, "0x%02x"%byte]
128127
retrytime = 6
129128
ret_t = ""
130129
for i in range(retrytime):
@@ -166,7 +165,7 @@ def readsysfs(location):
166165

167166
@staticmethod
168167
def getdevmem(addr, digit, mask):
169-
command_line = "devmem 0x%02x %d" %(addr, digit)
168+
command_line = ["devmem", "0x%02x"%addr, str(digit)]
170169
retrytime = 6
171170
ret_t = ""
172171
for i in range(retrytime):
@@ -179,13 +178,13 @@ def getdevmem(addr, digit, mask):
179178

180179
@staticmethod
181180
def rj_os_system(cmd):
182-
status, output = subprocess.getstatusoutput(cmd)
181+
status, output = getstatusoutput_noshell(cmd)
183182
return status, output
184183

185184
@staticmethod
186185
def getsdkreg(reg):
187186
try:
188-
cmd = "bcmcmd -t 1 'getr %s ' < /dev/null" % reg
187+
cmd = ["bcmcmd", "-t", "1", "getr"+str(reg)]
189188
ret, result = osutil.rj_os_system(cmd)
190189
result_t = result.strip().replace("\r", "").replace("\n", "")
191190
if ret != 0 or "Error:" in result_t:
@@ -203,8 +202,8 @@ def getmactemp():
203202
result = {}
204203
#waitForDocker()
205204
#need to exec twice
206-
osutil.rj_os_system("bcmcmd -t 1 \"show temp\" < /dev/null")
207-
ret, log = osutil.rj_os_system("bcmcmd -t 1 \"show temp\" < /dev/null")
205+
osutil.rj_os_system(["bcmcmd", "-t", "1", "show temp"])
206+
ret, log = osutil.rj_os_system(["bcmcmd", "-t", "1", "show temp"])
208207
if ret:
209208
return False, result
210209
else:

0 commit comments

Comments
 (0)