Skip to content

Commit

Permalink
[show] replace shell=True, replace xml by lxml, replace exit by sys.e…
Browse files Browse the repository at this point in the history
…xit (sonic-net#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 <maibui@microsoft.com>
  • Loading branch information
maipbui authored and pdhruv-marvell committed Aug 23, 2023
1 parent 9513e09 commit cc9ef65
Show file tree
Hide file tree
Showing 11 changed files with 534 additions and 189 deletions.
17 changes: 8 additions & 9 deletions show/bgp_quagga_v4.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import click
from show.main import AliasedGroup, ip, run_command
from show.main import ip, run_command
from utilities_common.bgp_util import get_bgp_summary_extended
import utilities_common.constants as constants
import utilities_common.cli as clicommon


###############################################################################
Expand All @@ -11,7 +12,7 @@
###############################################################################


@ip.group(cls=AliasedGroup)
@ip.group(cls=clicommon.AliasedGroup)
def bgp():
"""Show IPv4 BGP (Border Gateway Protocol) information"""
pass
Expand All @@ -22,10 +23,10 @@ def bgp():
def summary():
"""Show summarized information of IPv4 BGP state"""
try:
device_output = run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
device_output = run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ip bgp summary"], return_cmd=True)
get_bgp_summary_extended(device_output)
except Exception:
run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND))
run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ip bgp summary"])


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

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

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

# info_type is only valid if ipaddress is specified
if info_type is not None:
command += ' {}'.format(info_type)

command += '"'
command[-1] += ' {}'.format(info_type)

run_command(command)
11 changes: 6 additions & 5 deletions show/bgp_quagga_v6.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import click
from show.main import AliasedGroup, ipv6, run_command
from show.main import ipv6, run_command
from utilities_common.bgp_util import get_bgp_summary_extended
import utilities_common.constants as constants
import utilities_common.cli as clicommon


###############################################################################
Expand All @@ -11,7 +12,7 @@
###############################################################################


@ipv6.group(cls=AliasedGroup)
@ipv6.group(cls=clicommon.AliasedGroup)
def bgp():
"""Show IPv6 BGP (Border Gateway Protocol) information"""
pass
Expand All @@ -22,10 +23,10 @@ def bgp():
def summary():
"""Show summarized information of IPv6 BGP state"""
try:
device_output = run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
device_output = run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ipv6 bgp summary"], return_cmd=True)
get_bgp_summary_extended(device_output)
except Exception:
run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND))
run_command(['sudo', constants.RVTYSH_COMMAND, '-c', "show ipv6 bgp summary"])


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

0 comments on commit cc9ef65

Please sign in to comment.