From d122f1403c283149f6a59ecf6d39bf860cbe6612 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 16:01:14 +0400 Subject: [PATCH 01/23] Try to load the fallback cmd.exe by an absolute path --- Lib/subprocess.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 9cadd1bf8e622c..44e80521f0d769 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1480,7 +1480,16 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if shell: startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE - comspec = os.environ.get("COMSPEC", "cmd.exe") + # gh-101283: with no full path, Windows looks into a + # current directory first, so no plain "cmd.exe". + default_shell = "C:\\WINDOWS\\system32\\cmd.exe" + comspec = os.environ.get("COMSPEC", default_shell) + if not os.path.isfile(comspec): + # Windows is installed into a non-standard location + # or a system environment variable is broken. + # It's highly unlikely and we cannot help here. + comspec = "cmd.exe" + args = '{} /c "{}"'.format (comspec, args) if cwd is not None: From e96457c7220d9b8a6c347697d9a6691ae5f94a21 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 16:12:38 +0400 Subject: [PATCH 02/23] Add a NEWS entry --- .../Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst new file mode 100644 index 00000000000000..5a410ecfa0506d --- /dev/null +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -0,0 +1,3 @@ +:meth:`subprocess.Popen.__init__` no-``COMSPEC`` fallback now looks for an +absolute path ``C:\\WINDOWS\\system32\\cmd.exe`` before falling back further +to an old ``cmd.exe``. Patch by Oleg Iarygin. From b26918e0b8d87ec0d0f9178086ff1349acbf597f Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 16:35:59 +0400 Subject: [PATCH 03/23] Fix minor grammar --- Lib/subprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 44e80521f0d769..3b06ce5598ee9c 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1481,12 +1481,12 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE # gh-101283: with no full path, Windows looks into a - # current directory first, so no plain "cmd.exe". + # current directory first so no plain "cmd.exe". default_shell = "C:\\WINDOWS\\system32\\cmd.exe" comspec = os.environ.get("COMSPEC", default_shell) if not os.path.isfile(comspec): # Windows is installed into a non-standard location - # or a system environment variable is broken. + # or the system environment variable is broken. # It's highly unlikely and we cannot help here. comspec = "cmd.exe" From 9a0fdbd635885a31021473d2f55b931f21b3e52e Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 17:10:34 +0400 Subject: [PATCH 04/23] Address patchcheck CI error report --- Lib/subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 3b06ce5598ee9c..aecfd0ce2c347e 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1481,7 +1481,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE # gh-101283: with no full path, Windows looks into a - # current directory first so no plain "cmd.exe". + # current directory first so no plain "cmd.exe". default_shell = "C:\\WINDOWS\\system32\\cmd.exe" comspec = os.environ.get("COMSPEC", default_shell) if not os.path.isfile(comspec): From 091e970370d5694bd559a6c299eb40a658d06dda Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 19:51:57 +0400 Subject: [PATCH 05/23] Address the review Co-authored-by: Eryk Sun --- Lib/subprocess.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index aecfd0ce2c347e..95c6aa52ea466c 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1482,11 +1482,14 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, startupinfo.wShowWindow = _winapi.SW_HIDE # gh-101283: with no full path, Windows looks into a # current directory first so no plain "cmd.exe". - default_shell = "C:\\WINDOWS\\system32\\cmd.exe" - comspec = os.environ.get("COMSPEC", default_shell) + system_drive = os.environ.get('SystemDrive') or 'C:' + system_root = os.environ.get('SystemRoot') or os.path.join( + system_drive, 'Windows') + comspec = os.environ.get('ComSpec') or os.path.join( + system_root, 'System32', 'cmd.exe') if not os.path.isfile(comspec): # Windows is installed into a non-standard location - # or the system environment variable is broken. + # or the system environment variables are broken. # It's highly unlikely and we cannot help here. comspec = "cmd.exe" From a3cbdb67b5f2b7485e1907d43b035fc4f9c40471 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 19:48:25 +0300 Subject: [PATCH 06/23] Simplify NEWS wording and reattribute the totally rewritten PR --- .../2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst index 5a410ecfa0506d..1aeafa943b0e87 100644 --- a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -1,3 +1,6 @@ -:meth:`subprocess.Popen.__init__` no-``COMSPEC`` fallback now looks for an -absolute path ``C:\\WINDOWS\\system32\\cmd.exe`` before falling back further -to an old ``cmd.exe``. Patch by Oleg Iarygin. +Windows version of :meth:`subprocess.Popen.__init__` with +``shell=True`` made its shell search algorithm protected from dropping +a program named ``cmd.exe`` into a current directory. Now the current +directory is used as the last resort if system environment variables +are totally stripped and Windows is installed not into ``C\Windows``. +Patch by Eryk Sun. From 8dab7f110bcb8646f2b69fbd1192d314880bb291 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 19:49:25 +0300 Subject: [PATCH 07/23] Address the review-2 Co-authored-by: Eryk Sun --- Lib/subprocess.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 95c6aa52ea466c..af7a90949cbeb7 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1482,16 +1482,20 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, startupinfo.wShowWindow = _winapi.SW_HIDE # gh-101283: with no full path, Windows looks into a # current directory first so no plain "cmd.exe". - system_drive = os.environ.get('SystemDrive') or 'C:' - system_root = os.environ.get('SystemRoot') or os.path.join( - system_drive, 'Windows') - comspec = os.environ.get('ComSpec') or os.path.join( - system_root, 'System32', 'cmd.exe') - if not os.path.isfile(comspec): - # Windows is installed into a non-standard location - # or the system environment variables are broken. - # It's highly unlikely and we cannot help here. - comspec = "cmd.exe" + comspec = os.environ.get('ComSpec') + if not comspec: + system_drive = os.environ.get('SystemDrive') or 'C:' + system_root = os.environ.get('SystemRoot') or os.path.join( + system_drive, 'Windows') + comspec = os.path.join(system_root, 'System32', 'cmd.exe') + if not os.path.isfile(comspec): + # cmd.exe is missing, or the system environment + # variables are broken, or they're undefined and the + # system is installed into a non-standard location. + # This is highly unlikely, and we cannot help here. + comspec = 'cmd.exe' + if not executable and os.path.isabs(comspec): + executable = comspec args = '{} /c "{}"'.format (comspec, args) From 1d8d4a65303cadab3402a78e3221fb111546c9b6 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Tue, 24 Jan 2023 20:08:16 +0300 Subject: [PATCH 08/23] Update Lib/subprocess.py Co-authored-by: Eryk Sun --- Lib/subprocess.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index af7a90949cbeb7..9114ff5d47317f 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1480,8 +1480,11 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if shell: startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE - # gh-101283: with no full path, Windows looks into a - # current directory first so no plain "cmd.exe". + # gh-101283: without a fully-qualified path, before Windows + # checks the system directories, it first looks in the + # application directory, and also the current directory if + # NeedCurrentDirectoryForExePathW(ExeName) is true, so try + # to avoid executing unqualified "cmd.exe". comspec = os.environ.get('ComSpec') if not comspec: system_drive = os.environ.get('SystemDrive') or 'C:' From 9ed175825b546f31fd6cee4da02a6e501aa92db2 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 25 Jan 2023 08:05:17 +0400 Subject: [PATCH 09/23] Simplify the NEWS entry --- .../2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst index 1aeafa943b0e87..1f3c4858a1bd99 100644 --- a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -1,6 +1,2 @@ -Windows version of :meth:`subprocess.Popen.__init__` with -``shell=True`` made its shell search algorithm protected from dropping -a program named ``cmd.exe`` into a current directory. Now the current -directory is used as the last resort if system environment variables -are totally stripped and Windows is installed not into ``C\Windows``. -Patch by Eryk Sun. +Make ``shell=True`` mode of :meth:`subprocess.Popen.__init__` resilient +to a spoof of ``cmd.exe``. Patch by Eryk Sun. From a2734a66014b672d552143b017da06f1b86c9997 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 25 Jan 2023 08:10:18 +0400 Subject: [PATCH 10/23] Clarify attribution --- .../Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst index 1f3c4858a1bd99..c9072be26f0532 100644 --- a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -1,2 +1,3 @@ Make ``shell=True`` mode of :meth:`subprocess.Popen.__init__` resilient -to a spoof of ``cmd.exe``. Patch by Eryk Sun. +to spoofing of ``cmd.exe``. Patch by Eryk Sun, based on a path by Oleg +Iarygin. From 407aa355a2356322a2f04d315101b02413a04b43 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 25 Jan 2023 09:18:37 +0400 Subject: [PATCH 11/23] Update 2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst --- .../Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst index c9072be26f0532..be0b3da89d822b 100644 --- a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -1,3 +1,3 @@ -Make ``shell=True`` mode of :meth:`subprocess.Popen.__init__` resilient -to spoofing of ``cmd.exe``. Patch by Eryk Sun, based on a path by Oleg -Iarygin. +:class:`subprocess.Popen` now uses a safer approach to find +``cmd.exe`` when launching with ``shell=True``. Patch by Eryk Sun, +based on a path by Oleg Iarygin. From 880d443cdb4b9ace1afadba56a22baccae07aa69 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 28 Jan 2023 10:41:28 +0400 Subject: [PATCH 12/23] Add `Changed in version` entries --- Doc/library/subprocess.rst | 35 +++++++++++++++++++ ...-01-24-16-12-00.gh-issue-101283.9tqu39.rst | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 785251afdf262e..b5d1f06edfd220 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -111,6 +111,13 @@ underlying :class:`Popen` interface can be used directly. Added the *text* parameter, as a more understandable alias of *universal_newlines*. Added the *capture_output* parameter. + .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory is now used as the last resort insead of the first candidate. + As a result, dropping a malicious program named ``cmd.exe`` works only + when no other mechanism to locate ``cmd.exe`` is available. + .. class:: CompletedProcess The return value from :func:`run`, representing a process that has finished. @@ -483,6 +490,13 @@ functions. .. versionchanged:: 3.6 *executable* parameter accepts a :term:`path-like object` on POSIX. + .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory is now used as the last resort insead of the first candidate. + As a result, dropping a malicious program named ``cmd.exe`` works only + when no other mechanism to locate ``cmd.exe`` is available. + .. versionchanged:: 3.8 *executable* parameter accepts a bytes and :term:`path-like object` on Windows. @@ -1157,6 +1171,13 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. + .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory is now used as the last resort insead of the first candidate. + As a result, dropping a malicious program named ``cmd.exe`` works only + when no other mechanism to locate ``cmd.exe`` is available. + .. function:: check_call(args, *, stdin=None, stdout=None, stderr=None, \ shell=False, cwd=None, timeout=None, \ **other_popen_kwargs) @@ -1189,6 +1210,13 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. + .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory is now used as the last resort insead of the first candidate. + As a result, dropping a malicious program named ``cmd.exe`` works only + when no other mechanism to locate ``cmd.exe`` is available. + .. function:: check_output(args, *, stdin=None, stderr=None, shell=False, \ cwd=None, encoding=None, errors=None, \ @@ -1244,6 +1272,13 @@ calls these functions. .. versionadded:: 3.7 *text* was added as a more readable alias for *universal_newlines*. + .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory is now used as the last resort insead of the first candidate. + As a result, dropping a malicious program named ``cmd.exe`` works only + when no other mechanism to locate ``cmd.exe`` is available. + .. _subprocess-replacements: diff --git a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst index be0b3da89d822b..0efdfa10234185 100644 --- a/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst +++ b/Misc/NEWS.d/next/Security/2023-01-24-16-12-00.gh-issue-101283.9tqu39.rst @@ -1,3 +1,3 @@ :class:`subprocess.Popen` now uses a safer approach to find ``cmd.exe`` when launching with ``shell=True``. Patch by Eryk Sun, -based on a path by Oleg Iarygin. +based on a patch by Oleg Iarygin. From b34f998b6237e3f71819ae7386ca55c4dfc161da Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 28 Jan 2023 10:50:38 +0400 Subject: [PATCH 13/23] Try another multiversion syntax --- Doc/library/subprocess.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index b5d1f06edfd220..6c7ec96823dcff 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -111,7 +111,7 @@ underlying :class:`Popen` interface can be used directly. Added the *text* parameter, as a more understandable alias of *universal_newlines*. Added the *capture_output* parameter. - .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -490,7 +490,7 @@ functions. .. versionchanged:: 3.6 *executable* parameter accepts a :term:`path-like object` on POSIX. - .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1171,7 +1171,7 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1210,7 +1210,7 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1272,7 +1272,7 @@ calls these functions. .. versionadded:: 3.7 *text* was added as a more readable alias for *universal_newlines*. - .. versionchanged:: 3.7.17, 3.8.17, 3.9.17, 3.10.9, and 3.11.2 + .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. From 84cf94f31e533facbc0c48e4510c631694809d15 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Sat, 28 Jan 2023 11:08:15 +0400 Subject: [PATCH 14/23] Try the third, multiline syntax --- Doc/library/subprocess.rst | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 6c7ec96823dcff..b96b391c3abb26 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -111,7 +111,11 @@ underlying :class:`Popen` interface can be used directly. Added the *text* parameter, as a more understandable alias of *universal_newlines*. Added the *capture_output* parameter. - .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 + .. versionchanged:: 3.7.17 + .. versionchanged:: 3.8.17 + .. versionchanged:: 3.9.17 + .. versionchanged:: 3.10.9 + .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -490,7 +494,11 @@ functions. .. versionchanged:: 3.6 *executable* parameter accepts a :term:`path-like object` on POSIX. - .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 + .. versionchanged:: 3.7.17 + .. versionchanged:: 3.8.17 + .. versionchanged:: 3.9.17 + .. versionchanged:: 3.10.9 + .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1171,7 +1179,11 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 + .. versionchanged:: 3.7.17 + .. versionchanged:: 3.8.17 + .. versionchanged:: 3.9.17 + .. versionchanged:: 3.10.9 + .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1210,7 +1222,11 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 + .. versionchanged:: 3.7.17 + .. versionchanged:: 3.8.17 + .. versionchanged:: 3.9.17 + .. versionchanged:: 3.10.9 + .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. @@ -1272,7 +1288,11 @@ calls these functions. .. versionadded:: 3.7 *text* was added as a more readable alias for *universal_newlines*. - .. versionchanged:: 3.7.17 3.8.17 3.9.17 3.10.9 3.11.2 + .. versionchanged:: 3.7.17 + .. versionchanged:: 3.8.17 + .. versionchanged:: 3.9.17 + .. versionchanged:: 3.10.9 + .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current directory is now used as the last resort insead of the first candidate. From 9cbdfca886ae537ccaa9beb9ddea950146e906fa Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Mon, 30 Jan 2023 21:30:25 +0400 Subject: [PATCH 15/23] Apply the gpshead's suggestion --- Doc/library/subprocess.rst | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index b96b391c3abb26..7f0094048057ed 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -111,10 +111,6 @@ underlying :class:`Popen` interface can be used directly. Added the *text* parameter, as a more understandable alias of *universal_newlines*. Added the *capture_output* parameter. - .. versionchanged:: 3.7.17 - .. versionchanged:: 3.8.17 - .. versionchanged:: 3.9.17 - .. versionchanged:: 3.10.9 .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current @@ -494,10 +490,6 @@ functions. .. versionchanged:: 3.6 *executable* parameter accepts a :term:`path-like object` on POSIX. - .. versionchanged:: 3.7.17 - .. versionchanged:: 3.8.17 - .. versionchanged:: 3.9.17 - .. versionchanged:: 3.10.9 .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current @@ -1179,10 +1171,6 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17 - .. versionchanged:: 3.8.17 - .. versionchanged:: 3.9.17 - .. versionchanged:: 3.10.9 .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current @@ -1222,10 +1210,6 @@ calls these functions. .. versionchanged:: 3.3 *timeout* was added. - .. versionchanged:: 3.7.17 - .. versionchanged:: 3.8.17 - .. versionchanged:: 3.9.17 - .. versionchanged:: 3.10.9 .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current @@ -1288,10 +1272,6 @@ calls these functions. .. versionadded:: 3.7 *text* was added as a more readable alias for *universal_newlines*. - .. versionchanged:: 3.7.17 - .. versionchanged:: 3.8.17 - .. versionchanged:: 3.9.17 - .. versionchanged:: 3.10.9 .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current From 86473da43686ef5e046ba86cd1f1080f158e6697 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 17:10:44 +0400 Subject: [PATCH 16/23] Address the review --- Lib/subprocess.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 9114ff5d47317f..d720152a95e4e6 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1487,16 +1487,10 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, # to avoid executing unqualified "cmd.exe". comspec = os.environ.get('ComSpec') if not comspec: - system_drive = os.environ.get('SystemDrive') or 'C:' - system_root = os.environ.get('SystemRoot') or os.path.join( - system_drive, 'Windows') + system_root = os.environ.get('SystemRoot') comspec = os.path.join(system_root, 'System32', 'cmd.exe') if not os.path.isfile(comspec): - # cmd.exe is missing, or the system environment - # variables are broken, or they're undefined and the - # system is installed into a non-standard location. - # This is highly unlikely, and we cannot help here. - comspec = 'cmd.exe' + raise FileNotFoundError('cmd.exe not found; neither %ComSpec% nor %SystemRoot% is set') if not executable and os.path.isabs(comspec): executable = comspec From f1b441289adee5c8f0ffa16ac4251decbf282b24 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 21:31:05 +0400 Subject: [PATCH 17/23] Apply suggestions from code review Co-authored-by: Steve Dower --- Lib/subprocess.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index d720152a95e4e6..9236e6e71ffc74 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1487,9 +1487,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, # to avoid executing unqualified "cmd.exe". comspec = os.environ.get('ComSpec') if not comspec: - system_root = os.environ.get('SystemRoot') + system_root = os.environ.get('SystemRoot', '') comspec = os.path.join(system_root, 'System32', 'cmd.exe') - if not os.path.isfile(comspec): + if not os.path.isabs(comspec) or not os.path.isfile(comspec): raise FileNotFoundError('cmd.exe not found; neither %ComSpec% nor %SystemRoot% is set') if not executable and os.path.isabs(comspec): executable = comspec From 11ebd69699de26dfc0e0bb8816e45aab0a8a544c Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 21:41:25 +0400 Subject: [PATCH 18/23] Improve performance by triggering `cmd.exe` search on `executable=None` only --- Lib/subprocess.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 9236e6e71ffc74..9244a43a879b7f 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1480,19 +1480,20 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if shell: startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE - # gh-101283: without a fully-qualified path, before Windows - # checks the system directories, it first looks in the - # application directory, and also the current directory if - # NeedCurrentDirectoryForExePathW(ExeName) is true, so try - # to avoid executing unqualified "cmd.exe". - comspec = os.environ.get('ComSpec') - if not comspec: - system_root = os.environ.get('SystemRoot', '') - comspec = os.path.join(system_root, 'System32', 'cmd.exe') - if not os.path.isabs(comspec) or not os.path.isfile(comspec): - raise FileNotFoundError('cmd.exe not found; neither %ComSpec% nor %SystemRoot% is set') - if not executable and os.path.isabs(comspec): - executable = comspec + if not executable: + # gh-101283: without a fully-qualified path, before Windows + # checks the system directories, it first looks in the + # application directory, and also the current directory if + # NeedCurrentDirectoryForExePathW(ExeName) is true, so try + # to avoid executing unqualified "cmd.exe". + comspec = os.environ.get('ComSpec') + if not comspec: + system_root = os.environ.get('SystemRoot', '') + comspec = os.path.join(system_root, 'System32', 'cmd.exe') + if not os.path.isabs(comspec) or not os.path.isfile(comspec): + raise FileNotFoundError('cmd.exe not found; neither %ComSpec% nor %SystemRoot% is set') + if os.path.isabs(comspec): + executable = comspec args = '{} /c "{}"'.format (comspec, args) From 25dcbb887ccf4ad8fb488d20879b501d51bdf910 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 21:54:47 +0400 Subject: [PATCH 19/23] Update the Changed In section --- Doc/library/subprocess.rst | 43 +++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 7f0094048057ed..1d96244aec75f6 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -114,9 +114,10 @@ underlying :class:`Popen` interface can be used directly. .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current - directory is now used as the last resort insead of the first candidate. - As a result, dropping a malicious program named ``cmd.exe`` works only - when no other mechanism to locate ``cmd.exe`` is available. + directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and + `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + malicious program named ``cmd.exe`` into a current directory no + longer works. .. class:: CompletedProcess @@ -490,17 +491,18 @@ functions. .. versionchanged:: 3.6 *executable* parameter accepts a :term:`path-like object` on POSIX. - .. versionchanged:: 3.11.2 - - Changed Windows shell search order for ``shell=True``. The current - directory is now used as the last resort insead of the first candidate. - As a result, dropping a malicious program named ``cmd.exe`` works only - when no other mechanism to locate ``cmd.exe`` is available. - .. versionchanged:: 3.8 *executable* parameter accepts a bytes and :term:`path-like object` on Windows. + .. versionchanged:: 3.11.2 + + Changed Windows shell search order for ``shell=True``. The current + directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and + `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + malicious program named ``cmd.exe`` into a current directory no + longer works. + *stdin*, *stdout* and *stderr* specify the executed program's standard input, standard output and standard error file handles, respectively. Valid values are ``None``, :data:`PIPE`, :data:`DEVNULL`, an existing file descriptor (a @@ -1174,9 +1176,10 @@ calls these functions. .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current - directory is now used as the last resort insead of the first candidate. - As a result, dropping a malicious program named ``cmd.exe`` works only - when no other mechanism to locate ``cmd.exe`` is available. + directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and + `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + malicious program named ``cmd.exe`` into a current directory no + longer works. .. function:: check_call(args, *, stdin=None, stdout=None, stderr=None, \ shell=False, cwd=None, timeout=None, \ @@ -1213,9 +1216,10 @@ calls these functions. .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current - directory is now used as the last resort insead of the first candidate. - As a result, dropping a malicious program named ``cmd.exe`` works only - when no other mechanism to locate ``cmd.exe`` is available. + directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and + `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + malicious program named ``cmd.exe`` into a current directory no + longer works. .. function:: check_output(args, *, stdin=None, stderr=None, shell=False, \ @@ -1275,9 +1279,10 @@ calls these functions. .. versionchanged:: 3.11.2 Changed Windows shell search order for ``shell=True``. The current - directory is now used as the last resort insead of the first candidate. - As a result, dropping a malicious program named ``cmd.exe`` works only - when no other mechanism to locate ``cmd.exe`` is available. + directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and + `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + malicious program named ``cmd.exe`` into a current directory no + longer works. .. _subprocess-replacements: From fc08548e68cadbd56dca98920abb7e5548cd1e8d Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 22:29:09 +0400 Subject: [PATCH 20/23] Update Lib/subprocess.py Co-authored-by: Eryk Sun --- Lib/subprocess.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 9244a43a879b7f..ce1b28084680d3 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1490,8 +1490,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if not comspec: system_root = os.environ.get('SystemRoot', '') comspec = os.path.join(system_root, 'System32', 'cmd.exe') - if not os.path.isabs(comspec) or not os.path.isfile(comspec): - raise FileNotFoundError('cmd.exe not found; neither %ComSpec% nor %SystemRoot% is set') + if not os.path.isabs(comspec): + raise FileNotFoundError( + 'shell not found: neither %ComSpec% nor %SystemRoot% is set') if os.path.isabs(comspec): executable = comspec From 7e067039887cf76ede0d58a54dd9d1949b18e3d3 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 22:31:15 +0400 Subject: [PATCH 21/23] Fix env variable name misquoting From 81aba862e6506700857835b8283c9e7611fde766 Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 22:37:26 +0400 Subject: [PATCH 22/23] Manually fix env variable name misquoting --- Doc/library/subprocess.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index 1d96244aec75f6..6cd9803891d6c0 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -115,7 +115,7 @@ underlying :class:`Popen` interface can be used directly. Changed Windows shell search order for ``shell=True``. The current directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and - `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + ``%SystemRoot%\System32\cmd.exe``. As a result, dropping a malicious program named ``cmd.exe`` into a current directory no longer works. @@ -499,7 +499,7 @@ functions. Changed Windows shell search order for ``shell=True``. The current directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and - `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + ``%SystemRoot%\System32\cmd.exe``. As a result, dropping a malicious program named ``cmd.exe`` into a current directory no longer works. @@ -1177,7 +1177,7 @@ calls these functions. Changed Windows shell search order for ``shell=True``. The current directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and - `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + ``%SystemRoot%\System32\cmd.exe``. As a result, dropping a malicious program named ``cmd.exe`` into a current directory no longer works. @@ -1217,7 +1217,7 @@ calls these functions. Changed Windows shell search order for ``shell=True``. The current directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and - `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + ``%SystemRoot%\System32\cmd.exe``. As a result, dropping a malicious program named ``cmd.exe`` into a current directory no longer works. @@ -1280,7 +1280,7 @@ calls these functions. Changed Windows shell search order for ``shell=True``. The current directory and ``%PATH%`` are replaced with ``%COMSPEC%`` and - `%SystemRoot%\System32\cmd.exe``. As a result, dropping a + ``%SystemRoot%\System32\cmd.exe``. As a result, dropping a malicious program named ``cmd.exe`` into a current directory no longer works. From fecd606f709b8d5517558e0d42b99d67bd56289d Mon Sep 17 00:00:00 2001 From: Oleg Iarygin Date: Wed, 8 Feb 2023 23:02:17 +0400 Subject: [PATCH 23/23] Update Lib/subprocess.py Co-authored-by: Steve Dower --- Lib/subprocess.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index ce1b28084680d3..fa527d50ebb44d 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1491,8 +1491,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, system_root = os.environ.get('SystemRoot', '') comspec = os.path.join(system_root, 'System32', 'cmd.exe') if not os.path.isabs(comspec): - raise FileNotFoundError( - 'shell not found: neither %ComSpec% nor %SystemRoot% is set') + raise FileNotFoundError('shell not found: neither %ComSpec% nor %SystemRoot% is set') if os.path.isabs(comspec): executable = comspec