Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[show][barefoot] replace shell=True #2699

Merged
merged 4 commits into from
Apr 20, 2023

Conversation

maipbui
Copy link
Contributor

@maipbui maipbui commented Feb 28, 2023

Signed-off-by: maipbui 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

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Signed-off-by: maipbui <maibui@microsoft.com>
r'-maxdepth 1 -type d -name install_\*_profile ' + opts + '| sed '
r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%', shell=True)
cmd0 = ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\
r'-maxdepth', '1', r'-type', 'd', r'-name', r'install_\*_profile', opts]
Copy link
Contributor

@qiluo-msft qiluo-msft Feb 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r

This r is not needed. #Closed

r'-maxdepth 1 -type d -name install_\*_profile ' + opts + '| sed '
r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%', shell=True)
cmd0 = ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\
r'-maxdepth', '1', r'-type', 'd', r'-name', r'install_\*_profile', opts]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r

the same

r'-maxdepth 1 -type d -name install_\*_profile ' + opts + '| sed '
r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%', shell=True)
cmd0 = ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\
r'-maxdepth', '1', r'-type', 'd', r'-name', r'install_\*_profile', opts]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r

the same

Signed-off-by: maipbui <maibui@microsoft.com>
@maipbui
Copy link
Contributor Author

maipbui commented Mar 3, 2023

/azp run Azure.sonic-utilities

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Contributor

@oleksandrx-kolomeiets Could you help review?

qiluo-msft
qiluo-msft previously approved these changes Mar 7, 2023
Signed-off-by: maipbui <maibui@microsoft.com>
@maipbui
Copy link
Contributor Author

maipbui commented Mar 31, 2023

@oleksandrx-kolomeiets could you help review?

Signed-off-by: Mai Bui <maibui@microsoft.com>
@qiluo-msft qiluo-msft merged commit 88a7daa into sonic-net:master Apr 20, 2023
@maipbui maipbui deleted the show_barefoot_pysec branch April 20, 2023 18:16
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 1, 2023
Update sonic-utilities submodule pointer to include the following:
* 88ffb167 [config]config reload should generate sysinfo if missing ([sonic-net#2778](sonic-net/sonic-utilities#2778))
* 7443b9e5 [sonic-package-manager] support extension with multiple YANG modules ([sonic-net#2752](sonic-net/sonic-utilities#2752))
* 522c3a9e [sonic-package-manager] add support for multiple CLI plugin files ([sonic-net#2753](sonic-net/sonic-utilities#2753))
* b38fcfd1 [show][muxcable] fix  RC ([sonic-net#2812](sonic-net/sonic-utilities#2812))
* 7e24463f [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701))
* bee593e4 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802))
* ada603c5 [config]Support multi-asic  Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738))
* 88a7daa8 [show][barefoot] replace shell=True ([sonic-net#2699](sonic-net/sonic-utilities#2699))
* 5e99edb5 [sonic_package_manager] replace shell=True ([sonic-net#2726](sonic-net/sonic-utilities#2726))
* b547bb45 [acl-loader] Only add default deny rule when table is L3 or L3V6 ([sonic-net#2796](sonic-net/sonic-utilities#2796))

Signed-off-by: dprital <drorp@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants