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

Fix: address issues pointed out by Python scanners #3036

Merged
merged 5 commits into from
Nov 17, 2023

Conversation

tswhison
Copy link
Contributor

@tswhison tswhison commented Nov 6, 2023

Description

Our Python scanners point out issues regarding the use of
the subprocess module. The issues tend to be in 2 main
categories:

  1. The use of subprocess.Popen() directly is discouraged.
    Instead, the scanners recommend using run, call, or checked_call.

  2. The use of any of the subprocess calls with shell=True
    is forbidden, because it is prone to code injection attacks.
    Instead, the scanners require shell=False or omitting shell
    altogether.

Collateral (docs, reports, design examples, case IDs):

Python scans when preparing the release.

  • Document Update Required? (Specify FIM/AFU/Scripts)

Tests added:

Tests run:

Replacing it with pacsign-tests.sh. test.py contained several
bandit violations concerning the use of os.system() and assert.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Replace with subprocess.run(cmd, check=True) to appease python
scans.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Python scanners require that we avoid subprocess.Popen() and any
subprocess call which has shell=True. Use
subprocess.run(..., check=True) instead.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Python scan tools forbid subprocess.Popen() and any subprocess call
which uses shell=True.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
@tswhison tswhison self-assigned this Nov 6, 2023
@tswhison tswhison requested review from a team as code owners November 6, 2023 22:26
michael-adler
michael-adler previously approved these changes Nov 7, 2023
Copy link
Member

@michael-adler michael-adler left a comment

Choose a reason for hiding this comment

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

Tested an ASE config

anandaravuri
anandaravuri previously approved these changes Nov 7, 2023
@tswhison tswhison merged commit be8ff84 into release/2.6.0 Nov 17, 2023
20 checks passed
@tswhison tswhison deleted the tswhison/bandit_fixes branch November 17, 2023 19:41
tswhison added a commit that referenced this pull request Nov 17, 2023
### Description
Our Python scanners point out issues regarding the use of
the subprocess module. The issues tend to be in 2 main
categories:
1) The use of subprocess.Popen() directly is discouraged.
Instead, the scanners recommend using run, call, or checked_call.

2) The use of any of the subprocess calls with shell=True 
is forbidden, because it is prone to code injection attacks.
Instead, the scanners require shell=False or omitting shell
altogether.

### Collateral (docs, reports, design examples, case IDs):
Python scans when preparing the release.


- [ ] Document Update Required? (Specify FIM/AFU/Scripts)

### Tests added:


### Tests run:
CI and manual testing of rtl_src_config.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
tswhison added a commit that referenced this pull request Nov 17, 2023
### Description
Our Python scanners point out issues regarding the use of
the subprocess module. The issues tend to be in 2 main
categories:
1) The use of subprocess.Popen() directly is discouraged.
Instead, the scanners recommend using run, call, or checked_call.

2) The use of any of the subprocess calls with shell=True 
is forbidden, because it is prone to code injection attacks.
Instead, the scanners require shell=False or omitting shell
altogether.

### Collateral (docs, reports, design examples, case IDs):
Python scans when preparing the release.


- [ ] Document Update Required? (Specify FIM/AFU/Scripts)

### Tests added:


### Tests run:
CI and manual testing of rtl_src_config.

Signed-off-by: Tim Whisonant <tim.whisonant@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants