Fix: address issues pointed out by Python scanners (#3036)

### 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 <[email protected]>

binaries/fpgadiag/opae/diag/fecmode.py

@@ -1,5 +1,5 @@
 #! /usr/bin/env python3
-# Copyright(c) 2020, Intel Corporation
+# Copyright(c) 2020-2023, Intel Corporation
 #
 # Redistribution  and  use  in source  and  binary  forms,  with  or  without
 # modification, are permitted provided that the following conditions are met:
@@ -42,8 +42,8 @@
 CONF_FILE = '/etc/modprobe.d/dfl-fme.conf'
 OPTION_LINE = 'options dfl_n3000_nios fec_mode='
 DRV_MODE = '/sys/module/dfl_n3000_nios/parameters/fec_mode'
-REMOVE_MOD = 'rmmod dfl_n3000_nios'
-PROBE_MOD = 'modprobe dfl_n3000_nios'
+REMOVE_MOD = ['rmmod', 'dfl_n3000_nios']
+PROBE_MOD = ['modprobe', 'dfl_n3000_nios']
 
 
 def get_fpga_sysfs_path(sbdf):
@@ -85,14 +85,11 @@ def do_rsu(sbdf, debug):
         return None
 
     try:
-        cmd = "rsu bmcimg {}".format(sbdf)
+        cmd = ['rsu', 'bmcimg', sbdf]
         if debug:
+            cmd.append('-d')
             print(cmd)
-            cmd += ' -d'
-        rc = subprocess.call(cmd, shell=True)
-        if rc != 0:
-            print("failed to '{}'".format(cmd))
-            return None
+        subprocess.run(cmd, check=True)
     except subprocess.CalledProcessError as e:
         print('failed call')
         return None
@@ -145,10 +142,7 @@ def reload_driver(fec_mode, debug):
     try:
         if debug:
             print(REMOVE_MOD)
-        rc = subprocess.call(REMOVE_MOD, shell=True)
-        if rc != 0:
-            print("failed to '{}'".format(REMOVE_MOD))
-            return rc
+        subprocess.run(REMOVE_MOD, check=True)
     except subprocess.CalledProcessError as e:
         print('failed call')
         return 2
@@ -158,10 +152,7 @@ def reload_driver(fec_mode, debug):
     try:
         if debug:
             print(PROBE_MOD)
-        rc = subprocess.call(PROBE_MOD, shell=True)
-        if rc != 0:
-            print("failed to '{}'".format(PROBE_MOD))
-            return rc
+        subprocess.run(PROBE_MOD, check=True)
     except subprocess.CalledProcessError as e:
         print(e)
         return 2

binaries/fpgadiag/opae/diag/fpgadiag.py

@@ -1,5 +1,5 @@
 #! /usr/bin/env python3
-# Copyright(c) 2017-2020 Intel Corporation
+# Copyright(c) 2017-2023 Intel Corporation
 #
 # Redistribution  and  use  in source  and  binary  forms,  with  or  without
 # modification, are permitted provided that the following conditions are met:
@@ -67,10 +67,9 @@ def main():
 
     cmdline[0] = os.path.join(cwd, cmdline[0])
     cmdline = cmdline + ['-t', args.target] + leftover
-    cmdline = ' '.join(cmdline)
 
     try:
-        subprocess.check_call(cmdline, shell=True)
+        subprocess.run(cmdline, check=True)
     except CalledProcessError as e:
         exit(e.returncode)
 

binaries/fpgadiag/opae/diag/fpgastats.py

@@ -1,5 +1,5 @@
 #! /usr/bin/env python3
-# Copyright(c) 2018-2019, Intel Corporation
+# Copyright(c) 2018-2023, Intel Corporation
 #
 # Redistribution  and  use  in source  and  binary  forms,  with  or  without
 # modification, are permitted provided that the following conditions are met:
@@ -372,15 +372,13 @@ def eth_stats(self):
             print("Ethernet Interface Name:", eth_name[1])
             print("------------------------------")
             try:
-                cmd = "ethtool  {}".format(eth_name[1])
+                cmd = ['ethtool', eth_name[1]]
                 print(cmd)
-                rc = subprocess.call(cmd, shell=True)
-                cmd = "ethtool -S {}".format(eth_name[1])
+                subprocess.run(cmd, check=True)
+
+                cmd = ['ethtool', '-S', eth_name[1]]
                 print(cmd)
-                rc = subprocess.call(cmd, shell=True)
-                if rc != 0:
-                    print("failed to '{}'".format(cmd))
-                    return None
+                subprocess.run(cmd, check=True)
             except subprocess.CalledProcessError as e:
                 print('failed call')
                 return None

platforms/platmgr/tools/rtl_src_config.py

@@ -324,18 +324,16 @@ def addDefaultFpgaFamily(opts):
     if ('OPAE_PLATFORM_FPGA_FAMILY' not in os.environ):
         try:
             # Get the FPGA technology tag using afu_platform_info
-            cmd = 'afu_platform_info --key=fpga-family '
+            cmd = ['afu_platform_info', '--key=fpga-family']
 
             # What's the platform name?
             plat_class_file = os.path.join(getHWLibPath(opts),
                                            'fme-platform-class.txt')
             with open(plat_class_file) as f:
-                cmd += f.read().strip()
+                cmd.append(f.read().strip())
 
-            proc = subprocess.Popen(cmd, shell=True,
-                                    stdout=subprocess.PIPE)
-            for line in proc.stdout:
-                line = line.decode('ascii').strip()
+            proc = subprocess.run(cmd, check=True, capture_output=True, encoding='ascii')
+            for line in proc.stdout.split('\n'):
                 os.environ['OPAE_PLATFORM_FPGA_FAMILY'] = line
             errcode = proc.wait()
             if (errcode):
@@ -358,11 +356,10 @@ def getQuartusVersion(opts):
             'QUARTUS_VERSION_MAJOR' not in os.environ):
         try:
             # Get the Quartus major version number
-            proc = subprocess.Popen('quartus_sh --version', shell=True,
-                                    stdout=subprocess.PIPE)
+            cmd = ['quartus_sh', '--version']
+            proc = subprocess.run(cmd, check=True, capture_output=True, encoding='ascii')
             ok = False
-            for line in proc.stdout:
-                line = line.decode('ascii').strip()
+            for line in proc.stdout.split('\n'):
                 if (line[:7] == 'Version'):
                     ok = True
 

python/pacsign/ReadMe.txt

@@ -2,7 +2,11 @@ This is standalone signing tool
 
 You need to have Python 3.5/3.6 (tested) to run the script
 
-You can run test.py to fully execute all the available operation
+You can run pacsign-tests.sh to fully execute all the available operation
+
+    $ python3 -m virtualenv pacsign-venv
+    $ source ./pacsign-venv/bin/activate
+    $ pip3 install ./opae-sdk/python/pacsign
+    $ ./opae-sdk/python/pacsign/pacsign-tests.sh
+    $ deactivate
 
-    python test.py
-

python/pacsign/pacsign/common_util.py

@@ -96,22 +96,20 @@ def exception_handler(etype, value, tb):
 
 
 def run_command(command, printed_cmd=None, return_code=0, allow_error=False):
-
     if printed_cmd is None:
         printed_cmd = command
-    p = subprocess.Popen(
-        command,
-        shell=True,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        encoding="utf8",
-    )
-    returnmsg = p.communicate()[0]
-    assert p.returncode == 0 or allow_error, (
-        'Fail to run command "%s", error code %d =>\n%s'
-        % (printed_cmd, p.returncode, returnmsg)
-    )
-    return (p.returncode, returnmsg)
+
+    if isinstance(command, str):
+        command = command.split()
+
+    try:
+        p = subprocess.run(command, check=True, capture_output=True, encoding='ascii')
+    except subprocess.CalledProcessError:
+        assert allow_error, (
+            'Fail to run command "%s", error code %d =>\n%s'
+            % (printed_cmd, p.returncode, p.stderr)
+        )
+    return (p.returncode, p.stdout)
 
 
 def assert_in_error(boolean, string, *arg):