Skip to content

Commit a66f41c

Browse files
authored
[show] replace shell=True, replace xml by lxml, replace exit by sys.exit (#2666)
#### What I did `subprocess()` - when using with `shell=True` is dangerous. Using subprocess function without a static string can lead to command injection. `sys.exit` is better than `exit`, considered good to use in production code. Ref: https://stackoverflow.com/questions/6501121/difference-between-exit-and-sys-exit-in-python https://stackoverflow.com/questions/19747371/python-exit-commands-why-so-many-and-when-should-each-be-used #### How I did it `subprocess()` - use `shell=False` instead, use list of strings Ref: [https://semgrep.dev/docs/cheat-sheets/python-command-injection/#mitigation](https://semgrep.dev/docs/cheat-sheets/python-command-injection/#mitigation) Replace `exit()` by `sys.exit()` #### How to verify it Pass UT Manual test Signed-off-by: Mai Bui <[email protected]>
1 parent 5750057 commit a66f41c

11 files changed

+534
-189
lines changed

show/bgp_quagga_v4.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import click
2-
from show.main import AliasedGroup, ip, run_command
2+
from show.main import ip, run_command
33
from utilities_common.bgp_util import get_bgp_summary_extended
44
import utilities_common.constants as constants
5+
import utilities_common.cli as clicommon
56

67

78
###############################################################################
@@ -11,7 +12,7 @@
1112
###############################################################################
1213

1314

14-
@ip.group(cls=AliasedGroup)
15+
@ip.group(cls=clicommon.AliasedGroup)
1516
def bgp():
1617
"""Show IPv4 BGP (Border Gateway Protocol) information"""
1718
pass
@@ -22,10 +23,10 @@ def bgp():
2223
def summary():
2324
"""Show summarized information of IPv4 BGP state"""
2425
try:
25-
device_output = run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
26+
device_output = run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ip bgp summary"], return_cmd=True)
2627
get_bgp_summary_extended(device_output)
2728
except Exception:
28-
run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND))
29+
run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ip bgp summary"])
2930

3031

3132
# 'neighbors' subcommand ("show ip bgp neighbors")
@@ -35,15 +36,13 @@ def summary():
3536
def neighbors(ipaddress, info_type):
3637
"""Show IP (IPv4) BGP neighbors"""
3738

38-
command = 'sudo {} -c "show ip bgp neighbor'.format(constants.RVTYSH_COMMAND)
39+
command = ['sudo', constants.RVTYSH_COMMAND, '-c', "show ip bgp neighbor"]
3940

4041
if ipaddress is not None:
41-
command += ' {}'.format(ipaddress)
42+
command[-1] += ' {}'.format(ipaddress)
4243

4344
# info_type is only valid if ipaddress is specified
4445
if info_type is not None:
45-
command += ' {}'.format(info_type)
46-
47-
command += '"'
46+
command[-1] += ' {}'.format(info_type)
4847

4948
run_command(command)

show/bgp_quagga_v6.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import click
2-
from show.main import AliasedGroup, ipv6, run_command
2+
from show.main import ipv6, run_command
33
from utilities_common.bgp_util import get_bgp_summary_extended
44
import utilities_common.constants as constants
5+
import utilities_common.cli as clicommon
56

67

78
###############################################################################
@@ -11,7 +12,7 @@
1112
###############################################################################
1213

1314

14-
@ipv6.group(cls=AliasedGroup)
15+
@ipv6.group(cls=clicommon.AliasedGroup)
1516
def bgp():
1617
"""Show IPv6 BGP (Border Gateway Protocol) information"""
1718
pass
@@ -22,10 +23,10 @@ def bgp():
2223
def summary():
2324
"""Show summarized information of IPv6 BGP state"""
2425
try:
25-
device_output = run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
26+
device_output = run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ipv6 bgp summary"], return_cmd=True)
2627
get_bgp_summary_extended(device_output)
2728
except Exception:
28-
run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND))
29+
run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ipv6 bgp summary"])
2930

3031

3132
# 'neighbors' subcommand ("show ipv6 bgp neighbors")
@@ -34,5 +35,5 @@ def summary():
3435
@click.argument('info_type', type=click.Choice(['routes', 'advertised-routes', 'received-routes']), required=True)
3536
def neighbors(ipaddress, info_type):
3637
"""Show IPv6 BGP neighbors"""
37-
command = 'sudo {} -c "show ipv6 bgp neighbor {} {}"'.format(constants.RVTYSH_COMMAND, ipaddress, info_type)
38+
command = ['sudo', constants.RVTYSH_COMMAND, '-c', "show ipv6 bgp neighbor {} {}".format(ipaddress, info_type)]
3839
run_command(command)

0 commit comments

Comments
 (0)