Apply suggestions from code review

This commit is contained in:
Miss Islington (bot) 2023-02-08 15:45:17 -08:00 committed by GitHub
parent c63d7c95ba
commit 9889de3fa7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 1 deletions

View file

@ -110,6 +110,14 @@ 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.10.11
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.
.. class:: CompletedProcess
The return value from :func:`run`, representing a process that has finished.
@ -486,6 +494,14 @@ functions.
*executable* parameter accepts a bytes and :term:`path-like object`
on Windows.
.. versionchanged:: 3.10.11
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 :data:`PIPE`, :data:`DEVNULL`, an existing file descriptor (a positive
@ -1152,6 +1168,14 @@ calls these functions.
.. versionchanged:: 3.3
*timeout* was added.
.. versionchanged:: 3.10.11
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.
.. function:: check_call(args, *, stdin=None, stdout=None, stderr=None, \
shell=False, cwd=None, timeout=None, \
**other_popen_kwargs)
@ -1184,6 +1208,14 @@ calls these functions.
.. versionchanged:: 3.3
*timeout* was added.
.. versionchanged:: 3.10.11
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.
.. function:: check_output(args, *, stdin=None, stderr=None, shell=False, \
cwd=None, encoding=None, errors=None, \
@ -1239,6 +1271,14 @@ calls these functions.
.. versionadded:: 3.7
*text* was added as a more readable alias for *universal_newlines*.
.. versionchanged:: 3.10.11
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.
.. _subprocess-replacements:

View file

@ -1427,7 +1427,23 @@ class Popen:
if shell:
startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW
startupinfo.wShowWindow = _winapi.SW_HIDE
comspec = os.environ.get("COMSPEC", "cmd.exe")
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):
raise FileNotFoundError('shell not found: neither %ComSpec% nor %SystemRoot% is set')
if os.path.isabs(comspec):
executable = comspec
else:
comspec = executable
args = '{} /c "{}"'.format (comspec, args)
if cwd is not None:

View file

@ -0,0 +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 patch by Oleg Iarygin.