Skip to content

Commit ff032fe

Browse files
authoredApr 10, 2023
[debug/undebug] replace shell=True (sonic-net#2662)
Signed-off-by: Mai Bui <maibui@microsoft.com> #### What I did `subprocess()` - when using with `shell=True` is dangerous. Using subprocess function without a static string can lead to command injection. #### 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) #### How to verify it Manual test, execute all debug/undebug bgp/zebra commands Add UT
1 parent 882da01 commit ff032fe

File tree

3 files changed

+711
-104
lines changed

3 files changed

+711
-104
lines changed
 

‎debug/main.py

+55-53
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1+
import re
2+
import sys
13
import click
24
import subprocess
5+
from shlex import join
36

47
def run_command(command, pager=False):
5-
click.echo(click.style("Command: ", fg='cyan') + click.style(command, fg='green'))
6-
p = subprocess.Popen(command, shell=True, text=True, stdout=subprocess.PIPE)
8+
command_str = join(command)
9+
click.echo(click.style("Command: ", fg='cyan') + click.style(command_str, fg='green'))
10+
p = subprocess.Popen(command, text=True, stdout=subprocess.PIPE)
711
output = p.stdout.read()
812
if pager:
913
click.echo_via_pager(output)
@@ -21,8 +25,8 @@ def cli():
2125
"""SONiC command line - 'debug' command"""
2226
pass
2327

24-
25-
p = subprocess.check_output(["sudo vtysh -c 'show version'"], shell=True, text=True)
28+
prefix_pattern = '^[A-Za-z0-9.:/]*$'
29+
p = subprocess.check_output(['sudo', 'vtysh', '-c', 'show version'], text=True)
2630
if 'FRRouting' in p:
2731
#
2832
# 'bgp' group for FRR ###
@@ -35,89 +39,90 @@ def bgp():
3539
@bgp.command('allow-martians')
3640
def allow_martians():
3741
"""BGP allow martian next hops"""
38-
command = 'sudo vtysh -c "debug bgp allow-martians"'
42+
command = ['sudo', 'vtysh', '-c', "debug bgp allow-martians"]
3943
run_command(command)
4044

4145
@bgp.command()
4246
@click.argument('additional', type=click.Choice(['segment']), required=False)
4347
def as4(additional):
4448
"""BGP AS4 actions"""
45-
command = 'sudo vtysh -c "debug bgp as4'
49+
command = ['sudo', 'vtysh', '-c', "debug bgp as4"]
4650
if additional is not None:
47-
command += " segment"
48-
command += '"'
51+
command[-1] += " segment"
4952
run_command(command)
5053

5154
@bgp.command()
5255
@click.argument('prefix', required=True)
5356
def bestpath(prefix):
5457
"""BGP bestpath"""
55-
command = 'sudo vtysh -c "debug bgp bestpath %s"' % prefix
58+
if not re.match(prefix_pattern, prefix):
59+
sys.exit('Prefix contains only number, alphabet, period, colon, and forward slash')
60+
command = ['sudo', 'vtysh', '-c', "debug bgp bestpath %s" % prefix]
5661
run_command(command)
5762

5863
@bgp.command()
5964
@click.argument('prefix_or_iface', required=False)
6065
def keepalives(prefix_or_iface):
6166
"""BGP Neighbor Keepalives"""
62-
command = 'sudo vtysh -c "debug bgp keepalives'
67+
command = ['sudo', 'vtysh', '-c', "debug bgp keepalives"]
6368
if prefix_or_iface is not None:
64-
command += " " + prefix_or_iface
65-
command += '"'
69+
command[-1] += ' ' + prefix_or_iface
6670
run_command(command)
6771

6872
@bgp.command('neighbor-events')
6973
@click.argument('prefix_or_iface', required=False)
7074
def neighbor_events(prefix_or_iface):
7175
"""BGP Neighbor Events"""
72-
command = 'sudo vtysh -c "debug bgp neighbor-events'
76+
command = ['sudo', 'vtysh', '-c', "debug bgp neighbor-events"]
7377
if prefix_or_iface is not None:
74-
command += " " + prefix_or_iface
75-
command += '"'
78+
command[-1] += ' ' + prefix_or_iface
7679
run_command(command)
7780

7881
@bgp.command()
7982
def nht():
8083
"""BGP nexthop tracking events"""
81-
command = 'sudo vtysh -c "debug bgp nht"'
84+
command = ['sudo', 'vtysh', '-c', "debug bgp nht"]
8285
run_command(command)
8386

8487
@bgp.command()
8588
@click.argument('additional', type=click.Choice(['error']), required=False)
8689
def pbr(additional):
8790
"""BGP policy based routing"""
88-
command = 'sudo vtysh -c "debug bgp pbr'
91+
command = ['sudo', 'vtysh', '-c', "debug bgp pbr"]
8992
if additional is not None:
90-
command += " error"
91-
command += '"'
93+
command[-1] += " error"
9294
run_command(command)
9395

9496
@bgp.command('update-groups')
9597
def update_groups():
9698
"""BGP update-groups"""
97-
command = 'sudo vtysh -c "debug bgp update-groups"'
99+
command = ['sudo', 'vtysh', '-c', "debug bgp update-groups"]
98100
run_command(command)
99101

100102
@bgp.command()
101103
@click.argument('direction', type=click.Choice(['in', 'out', 'prefix']), required=False)
102104
@click.argument('prefix', required=False)
103105
def updates(direction, prefix):
104106
"""BGP updates"""
105-
command = 'sudo vtysh -c "debug bgp updates'
107+
bgp_cmd = "debug bgp updates"
106108
if direction is not None:
107-
command += " " + direction
109+
bgp_cmd += ' ' + direction
108110
if prefix is not None:
109-
command += " " + prefix
110-
command += '"'
111+
if not re.match(prefix_pattern, prefix):
112+
sys.exit('Prefix contains only number, alphabet, period, colon, and forward slash')
113+
bgp_cmd += ' ' + prefix
114+
command = ['sudo', 'vtysh', '-c', bgp_cmd]
111115
run_command(command)
112116

113117
@bgp.command()
114118
@click.argument('prefix', required=False)
115119
def zebra(prefix):
116120
"""BGP Zebra messages"""
117-
command = 'sudo vtysh -c "debug bgp zebra'
121+
command = ['sudo', 'vtysh', '-c', "debug bgp zebra"]
118122
if prefix is not None:
119-
command += " prefix " + prefix
120-
command += '"'
123+
if not re.match(prefix_pattern, prefix):
124+
sys.exit('Prefix contains only number, alphabet, period, colon, and forward slash')
125+
command[-1] += " prefix " + prefix
121126
run_command(command)
122127

123128
#
@@ -132,56 +137,54 @@ def zebra():
132137
@click.argument('detailed', type=click.Choice(['detailed']), required=False)
133138
def dplane(detailed):
134139
"""Debug zebra dataplane events"""
135-
command = 'sudo vtysh -c "debug zebra dplane'
140+
command = ['sudo', 'vtysh', '-c', "debug zebra dplane"]
136141
if detailed is not None:
137-
command += " detailed"
138-
command += '"'
142+
command[-1] += " detailed"
139143
run_command(command)
140144

141145
@zebra.command()
142146
def events():
143147
"""Debug option set for zebra events"""
144-
command = 'sudo vtysh -c "debug zebra events"'
148+
command = ['sudo', 'vtysh', '-c', "debug zebra events"]
145149
run_command(command)
146150

147151
@zebra.command()
148152
def fpm():
149153
"""Debug zebra FPM events"""
150-
command = 'sudo vtysh -c "debug zebra fpm"'
154+
command = ['sudo', 'vtysh', '-c', "debug zebra fpm"]
151155
run_command(command)
152156

153157
@zebra.command()
154158
def kernel():
155159
"""Debug option set for zebra between kernel interface"""
156-
command = 'sudo vtysh -c "debug zebra kernel"'
160+
command = ['sudo', 'vtysh', '-c', "debug zebra kernel"]
157161
run_command(command)
158162

159163
@zebra.command()
160164
def nht():
161165
"""Debug option set for zebra next hop tracking"""
162-
command = 'sudo vtysh -c "debug zebra nht"'
166+
command = ['sudo', 'vtysh', '-c', "debug zebra nht"]
163167
run_command(command)
164168

165169
@zebra.command()
166170
def packet():
167171
"""Debug option set for zebra packet"""
168-
command = 'sudo vtysh -c "debug zebra packet"'
172+
command = ['sudo', 'vtysh', '-c', "debug zebra packet"]
169173
run_command(command)
170174

171175
@zebra.command()
172176
@click.argument('detailed', type=click.Choice(['detailed']), required=False)
173177
def rib(detailed):
174178
"""Debug RIB events"""
175-
command = 'sudo vtysh -c "debug zebra rib'
179+
command = ['sudo', 'vtysh', '-c', "debug zebra rib"]
176180
if detailed is not None:
177-
command += " detailed"
178-
command += '"'
181+
command[-1] += " detailed"
179182
run_command(command)
180183

181184
@zebra.command()
182185
def vxlan():
183186
"""Debug option set for zebra VxLAN (EVPN)"""
184-
command = 'sudo vtysh -c "debug zebra vxlan"'
187+
command = ['sudo', 'vtysh', '-c', "debug zebra vxlan"]
185188
run_command(command)
186189

187190
else:
@@ -193,49 +196,49 @@ def vxlan():
193196
def bgp(ctx):
194197
"""debug bgp on"""
195198
if ctx.invoked_subcommand is None:
196-
command = 'sudo vtysh -c "debug bgp"'
199+
command = ['sudo', 'vtysh', '-c', "debug bgp"]
197200
run_command(command)
198201

199202
@bgp.command()
200203
def events():
201204
"""debug bgp events on"""
202-
command = 'sudo vtysh -c "debug bgp events"'
205+
command = ['sudo', 'vtysh', '-c', "debug bgp events"]
203206
run_command(command)
204207

205208
@bgp.command()
206209
def updates():
207210
"""debug bgp updates on"""
208-
command = 'sudo vtysh -c "debug bgp updates"'
211+
command = ['sudo', 'vtysh', '-c', "debug bgp updates"]
209212
run_command(command)
210213

211214
@bgp.command()
212215
def as4():
213216
"""debug bgp as4 actions on"""
214-
command = 'sudo vtysh -c "debug bgp as4"'
217+
command = ['sudo', 'vtysh', '-c', "debug bgp as4"]
215218
run_command(command)
216219

217220
@bgp.command()
218221
def filters():
219222
"""debug bgp filters on"""
220-
command = 'sudo vtysh -c "debug bgp filters"'
223+
command = ['sudo', 'vtysh', '-c', "debug bgp filters"]
221224
run_command(command)
222225

223226
@bgp.command()
224227
def fsm():
225228
"""debug bgp finite state machine on"""
226-
command = 'sudo vtysh -c "debug bgp fsm"'
229+
command = ['sudo', 'vtysh', '-c', "debug bgp fsm"]
227230
run_command(command)
228231

229232
@bgp.command()
230233
def keepalives():
231234
"""debug bgp keepalives on"""
232-
command = 'sudo vtysh -c "debug bgp keepalives"'
235+
command = ['sudo', 'vtysh', '-c', "debug bgp keepalives"]
233236
run_command(command)
234237

235238
@bgp.command()
236239
def zebra():
237240
"""debug bgp zebra messages on"""
238-
command = 'sudo vtysh -c "debug bgp zebra"'
241+
command = ['sudo', 'vtysh', '-c', "debug bgp zebra"]
239242
run_command(command)
240243

241244
#
@@ -248,32 +251,31 @@ def zebra():
248251

249252
@zebra.command()
250253
def events():
251-
"""debug option set for zebra events"""
252-
command = 'sudo vtysh -c "debug zebra events"'
254+
command = ['sudo', 'vtysh', '-c', "debug zebra events"]
253255
run_command(command)
254256

255257
@zebra.command()
256258
def fpm():
257259
"""debug zebra FPM events"""
258-
command = 'sudo vtysh -c "debug zebra fpm"'
260+
command = ['sudo', 'vtysh', '-c', "debug zebra fpm"]
259261
run_command(command)
260262

261263
@zebra.command()
262264
def kernel():
263265
"""debug option set for zebra between kernel interface"""
264-
command = 'sudo vtysh -c "debug zebra kernel"'
266+
command = ['sudo', 'vtysh', '-c', "debug zebra kernel"]
265267
run_command(command)
266268

267269
@zebra.command()
268270
def packet():
269271
"""debug option set for zebra packet"""
270-
command = 'sudo vtysh -c "debug zebra packet"'
272+
command = ['sudo', 'vtysh', '-c', "debug zebra packet"]
271273
run_command(command)
272274

273275
@zebra.command()
274276
def rib():
275277
"""debug RIB events"""
276-
command = 'sudo vtysh -c "debug zebra rib"'
278+
command = ['sudo', 'vtysh', '-c', "debug zebra rib"]
277279
run_command(command)
278280

279281

0 commit comments

Comments
 (0)