Feb 8, 2023 · Jan 24, 2023 · Jan 24, 2023 · Jan 24, 2023 · Jan 24, 2023 · Jan 24, 2023
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
            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".
                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)

            if cwd is not None:
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
 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.
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1480,7 +1480,23 @@ 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".
arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		comspec = os.environ.get('ComSpec')
eryksun marked this conversation as resolved. OutdatedShow resolvedHide resolved
		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')
arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		if not os.path.isfile(comspec):
zooba marked this conversation as resolved. OutdatedShow resolvedHide resolved arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		# 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.
Copy link Member gpsheadJan 24, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Should a RuntimeWarning be raised or logging.warn call made in this case as an FYI? Copy link Contributor eryksunJan 24, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. That's a good idea. How about this? comspec='cmd.exe'warnings.warn(f'spawning "{comspec}" using a ''relative file path',RuntimeWarning) Copy link Member gpsheadJan 25, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. if we keep the fallback at all rather than just make it an error, something like that. probably with a stacklevel= set. related to my other comment, i wouldn't want to add a new warning in a security backport either.
		comspec = 'cmd.exe'
		if not executable and os.path.isabs(comspec):
arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		executable = comspec

		args = '{} /c "{}"'.format (comspec, args)

		if cwd is not None:
Expand Down
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,6 @@
		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
arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		are totally stripped and Windows is installed not into ``C\Windows``.
arhadthedev marked this conversation as resolved. OutdatedShow resolvedHide resolved
		Patch by Eryk Sun.
eryksun marked this conversation as resolved. OutdatedShow resolvedHide resolved Copy link Member zoobaJan 24, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. This is the kind of too-much-detail I was concerned about in my review comments. Let's keep the NEWS entry simple and update the docs, but make sure we retain the ability to use an even more reliable algorithm in the future. gpshead and arhadthedev reacted with thumbs up emoji