[device/quanta] Mitigation for security vulnerability

[maipbui]

Signed-off-by: maipbui maibui@microsoft.com

Dependency: #12065

Why I did it

shell=True is dangerous because this call will spawn the command using a shell process
os - not secure against maliciously constructed input and dangerous if used to evaluate dynamic content.

How I did it

os - use with subprocess
Use shell=False with shell features

• redirection: https://stackoverflow.com/questions/4965159/how-to-redirect-output-with-subprocess-in-python/6482200#6482200?newreg=53afb91b3ebd47c5930be627fcdf2930
• | operator: https://docs.python.org/2/library/subprocess.html#replacing-shell-pipeline

How to verify it

Which release branch to backport (provide reason below if selected)

• 201811
• 201911
• 202006
• 202012
• 202106
• 202111
• 202205

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

[qiluo-msft]

def log_os_system(cmd, show):

def log_os_system(cmd1args, cmd2args, show):

---

In reply to: 1231957757

---

In reply to: 1231957757

---

In reply to: 1231957757

---

Refers to: device/quanta/x86_64-quanta_ix1b_rglbmc-r0/plugins/psuutil.py:47 in ab31cfe. [](commit_id = ab31cfe, deletion_comment = False)

[yejianquan]

/AzurePipelines run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[qiluo-msft]

As comments

[maipbui]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[maipbui]

/azp run Azure.sonic-buildimage

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lgtm-com]

This pull request fixes 2 alerts when merging b2f432b into e662008 - view on LGTM.com

fixed alerts:

• 2 for Unused import

[lgtm-com]

This pull request fixes 2 alerts when merging a6453a5 into e662008 - view on LGTM.com

fixed alerts:

• 2 for Unused import

[lgtm-com]

This pull request introduces 7 alerts and fixes 2 when merging ba7a4be into 1f0699f - view on LGTM.com

new alerts:

• 7 for Comparison using is when operands support `__eq__`

fixed alerts:

• 2 for Unused import

[maipbui]

@roberthong-qct @jonathantsai-qci Could you help review and verify?

[roberthong-qct]

Hi @maipbui , thanks for the security enhancement.
However, after I build your branch and try "show platform firmware" and "show interfaces transceiver eeprom" on IX8A_BDE, the error messages show as below:
"ImportError: cannot import name 'getstatusoutput_noshell_pipe' from 'sonic_py_common.general' (/usr/local/lib/python3.9/dist-packages/sonic_py_common/general.py)- required module not found"

[maipbui]

Hi @maipbui , thanks for the security enhancement. However, after I build your branch and try "show platform firmware" and "show interfaces transceiver eeprom" on IX8A_BDE, the error messages show as below: "ImportError: cannot import name 'getstatusoutput_noshell_pipe' from 'sonic_py_common.general' (/usr/local/lib/python3.9/dist-packages/sonic_py_common/general.py)- required module not found"

Hi @roberthong-qct, could you try install the latest sonic_py_common package first? The 'getstatusoutput_noshell_pipe' function is implemented in this PR #12065.

[maipbui]

@roberthong-qct could you update on your verification?

[lgtm-com]

This pull request fixes 2 alerts when merging 62c4237 into a750930 - view on LGTM.com

fixed alerts:

• 2 for Unused import

[roberthong-qct]

@roberthong-qct could you update on your verification?

_get_command_result_pipe() in component.py has a problem checking the return values of getstatusoutput_noshell_pipe()
,and there's a missing comma in comopent.py for IX7.
Please refer to the attachment for my modifications, thank you.
component_diff.txt

[maipbui]

@roberthong-qct could you update on your verification?

_get_command_result_pipe() in component.py has a problem checking the return values of getstatusoutput_noshell_pipe() ,and there's a missing comma in comopent.py for IX7. Please refer to the attachment for my modifications, thank you. component_diff.txt

Thanks @roberthong-qct ! I have fixed all issues based on your attachment. Is it good to merge?

[lgtm-com]

This pull request fixes 2 alerts when merging e521b22 into bc8ee7a - view on LGTM.com

fixed alerts:

• 2 for Unused import

[roberthong-qct]

Thanks @roberthong-qct ! I have fixed all issues based on your attachment. Is it good to merge?

@maipbui Yes, relevant daemons and commands are running well.