Undocumented risky behaviour in subprocess module

[mauricelambert]

Bug report - Undocumented risky behaviour in subprocess module

When using subprocess.Popen with shell=True on Windows and without a COMSPEC environment variable, a cmd.exe is launched. The problem is the cmd.exe full path is not written, Windows will search the executable in the current directory and in the PATH. If an arbitrary executable file is written to the current directory or to a directory in the PATH, it can be run instead of the real cmd.exe.

See the code here and a POC here.

• This risky behaviour can be patched by replacing cmd.exe string by C:\WINDOWS\system32\cmd.exe.
• If the behavior was chosen by python developers, it should be documented.

Linked PRs

• gh-101283: Try to load the fallback cmd.exe by an absolute path #101286
• [3.10] gh-101283: Improved fallback logic for subprocess with shell=True on Windows (GH-101286) #101708
• [3.9] gh-101283: Improved fallback logic for subprocess with shell=True on Windows (GH-101286) #101709
• [3.8] gh-101283: Improved fallback logic for subprocess with shell=True on Windows (GH-101286) #101710
• [3.11] gh-101283: Improved fallback logic for subprocess with shell=True on Windows (GH-101286) #101711
• gh-101283: Fix use of unbound variable #101712
• [3.7] gh-101283: Improved fallback logic for subprocess with shell=True on Windows (GH-101286) #101713
• gh-101283: Version was just released, so should be changed in 3.11.3 #101719
• [3.11] gh-101283: Version was just released, so should be changed in 3.11.3 (GH-101719) #101721
• gh-101283: Fix the versionchanged of gh-101283 (3.12 only) #101728

[eryksun]

FYI, subprocess.Popen should never find "cmd.exe" in PATH. We call CreateProcessW(), which always searches the application directory and system directories before it searches PATH.

CreateProcessW() may also check the current directory immediately after the application directory and before the system directories. This unsafe behavior can be prevented by defining the environment variable NoDefaultCurrentDirectoryInExePath. This environment variable has been supported for over 15 years now, since Windows Vista. Our implementation of shutil.which() should honor this security feature by calling NeedCurrentDirectoryForExePathW().

Note that if NoDefaultCurrentDirectoryInExePath is defined, the current directory is still included in the search path if the name is a relative path that includes backslashes (e.g. "Scripts\eggs.exe"), but not if it only includes forward slashes (e.g. "Scripts/eggs.exe"). Note also that the application directory takes precedence over the current directory in this case (e.g. if the application directory is "C:\Program Files\Python311", then "Scripts\eggs.exe" will resolve to "C:\Program Files\Python311\Scripts\eggs.exe" if it exists).

If you define NoDefaultCurrentDirectoryInExePath, the current directory can be added explicitly as a "." entry at the end of either the system or user PATH. I add "." to the end of the user PATH because some apps and scripts still depend on finding binaries and scripts in the current directory, but I won't cater to a bad actor that expects the current directory to be searched before system directories.

[gpshead]

FYI - this originally came in as a security report to PSRT. I redirected the reporter here (thanks for reporting this!) as this didn't seem serious enough to be embargoed. I'm still tagging it as such and aiming for a full set of backports as it feels like a good idea.

[gpshead]

The versionchanged markers in several of the PRs are inaccurate.

[zooba]

The backports are not merged yet - do you want to just fix it there?

[zooba]

Ah I see, it's only the versions in main and 3.11 branches.

[zooba]

Fully merged, and all the fixes should be in (I hope). Thanks all!