Safe usage of popen (#6490)

Avoid shell=True security issues with Popen
microsoft · Sep 4, 2024 · 662a421 · 662a421
1 parent ddd3571
commit 662a421
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 10 deletions.
diff --git a/deepspeed/utils/numa.py b/deepspeed/utils/numa.py
@@ -49,8 +49,8 @@ def check_for_numactl_pkg():
         flag, lib, tool = data
         path = distutils.spawn.find_executable(pkgmgr)
         if path is not None:
-            cmd = f"{pkgmgr} {flag} {lib}"
-            result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+            cmd = [pkgmgr, flag, lib]
+            result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             if result.wait() == 0:
                 found = True
             else:

diff --git a/op_builder/async_io.py b/op_builder/async_io.py
@@ -81,8 +81,8 @@ def check_for_libaio_pkg(self):
             flag, lib, tool = data
             path = distutils.spawn.find_executable(pkgmgr)
             if path is not None:
-                cmd = f"{pkgmgr} {flag} {lib}"
-                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+                cmd = [pkgmgr, flag, lib]
+                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                 if result.wait() == 0:
                     found = True
                 else:

diff --git a/op_builder/builder.py b/op_builder/builder.py
@@ -482,7 +482,8 @@ def command_exists(self, cmd):
             cmds = [cmd]
         valid = False
         for cmd in cmds:
-            result = subprocess.Popen(f'type {cmd}', stdout=subprocess.PIPE, shell=True)
+            safe_cmd = ["bash", "-c", f"type {cmd}"]
+            result = subprocess.Popen(safe_cmd, stdout=subprocess.PIPE)
             valid = valid or result.wait() == 0
 
         if not valid and len(cmds) > 1:

diff --git a/op_builder/npu/async_io.py b/op_builder/npu/async_io.py
@@ -74,8 +74,8 @@ def check_for_libaio_pkg(self):
             flag, lib, tool = data
             path = distutils.spawn.find_executable(pkgmgr)
             if path is not None:
-                cmd = f"{pkgmgr} {flag} {lib}"
-                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+                cmd = [pkgmgr, flag, lib]
+                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                 if result.wait() == 0:
                     found = True
                 else:

diff --git a/op_builder/xpu/async_io.py b/op_builder/xpu/async_io.py
@@ -70,8 +70,8 @@ def check_for_libaio_pkg(self):
             flag, lib, tool = data
             path = distutils.spawn.find_executable(pkgmgr)
             if path is not None:
-                cmd = f"{pkgmgr} {flag} {lib}"
-                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+                cmd = [pkgmgr, flag, lib]
+                result = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
                 if result.wait() == 0:
                     found = True
                 else:

diff --git a/setup.py b/setup.py
@@ -160,7 +160,8 @@ def command_exists(cmd):
         result = subprocess.Popen(f'{cmd}', stdout=subprocess.PIPE, shell=True)
         return result.wait() == 1
     else:
-        result = subprocess.Popen(f'type {cmd}', stdout=subprocess.PIPE, shell=True)
+        safe_cmd = ["bash", "-c", f"type {cmd}"]
+        result = subprocess.Popen(safe_cmd, stdout=subprocess.PIPE)
         return result.wait() == 0