bpo-30730: Prevent environment variables injection in subprocess on Windows.

[serhiy-storchaka]

Prevent passing other environment variables and command arguments.

[eryksun]

This search should start at index 1. On Windows, the equals character is allowed as the first character of an environment variable name. It's used for hidden environment variables such as per-drive working directories, e.g. =E:=E:\working\directory.

Really, Windows Python has a deficiency in that os.environ is built from the CRT's environ, which doesn't include these hidden variables. If a script uses os.environ.copy() or nt.environ.copy() as the basis for the env parameter, then it loses the per-drive working directories (except for the process current directory). For example:

>>> os.chdir(r'E:\working\directory')
>>> os.chdir(r'C:\Temp')
>>> args = ['python', '-c', "import os; print(os.path.abspath('E:test'))"]

>>> r = subprocess.call(args)
E:\working\directory\test

>>> environ = nt.environ.copy()
>>> r = subprocess.call(args, env=environ)
E:\test

>>> environ['=E:'] = r'E:\working\directory'
>>> r = subprocess.call(args, env=environ)
E:\working\directory\test

[serhiy-storchaka]

Thank you! Where can I read about this?

[eryksun]

As far as I know you won't find anything helpful on MSDN. Their article on changing environment variables says that equals cannot be used in the name, without explaining its use as the first character. Also, their article on changing the current directory incorrectly states that SetCurrentDirectory remembers per-drive directories, which it definitely never does.

The Windows API consumes these variables when resolving paths, but it never sets them. That's left to applications, and usually via the C runtime chdir function. Since Python has its own implementation of chdir on Windows, you can see how it sets these variables in win32_wchdir.

[bedevere-bot]

GH-2360 is a backport of this pull request to the 3.6 branch.

[bedevere-bot]

GH-2361 is a backport of this pull request to the 3.5 branch.

[bedevere-bot]

GH-2372 is a backport of this pull request to the 2.7 branch.

[mattip]

@serhiy-storchaka I am curious about this. Is there a particular version of Windows this works on? We copied this code in PyPy, but it is failing tests for putenv("=hidden", "foo")

[eryksun]

@mattip, the system tracks the working directory on each drive using environment variable names that start with "=", such as "=C:". Python's os.chdir() implements this. Such names are accessible via WinAPI GetEnvironmentStringsW, GetEnvironmentVariableW, and SetEnvironmentVariableW. In most contexts such variables are hidden or filtered out, and using them at the application level isn't officially supported. In particular, CRT environment functions such as putenv and getenv do not support them, and they're filtered out of C [_w]environ, from which Python initializes os.environ.

Here's a manual example:

>>> os.path.abspath('Z:')
'Z:\\'
>>> win32api.SetEnvironmentVariable('=Z:', 'Z:\\test_cwd')
>>> os.path.abspath('Z:')
'Z:\\test_cwd'

Or set the drive working directory in the environment of a child process:

>>> os.path.abspath('Z:')
'Z:\\'
>>> environ = os.environ.copy()
>>> environ['=Z:'] = 'Z:\\test_cwd'
>>> subprocess.call('python -c "import os;print(os.path.abspath(\'Z:\'))"', env=environ)
Z:\test_cwd
0

[mattip]

Thanks for the quick reply. So if I understand correctly while the code (in three places: this one and twice in posix_module) may permit the use of =hidden, it is not guaranteed that the subsequent system call will succeed.

[eryksun]

Names that begin with "=" always work with the Windows functions GetEnvironmentVariableW and SetEnvironmentVariableW, and they're inherited fine by child processes. For example:

>>> win32api.SetEnvironmentVariable('=spam', 'eggs')
>>> win32api.GetEnvironmentVariable('=spam')
'eggs'
>>> subprocess.call('python -c "import win32api; print(win32api.GetEnvironmentVariable(\'=spam\'))"')
eggs
0

However, such names are not officially supported. IMO, this is primarily because the C runtime filters them out of its [_w]environ global variable and doesn't support them in environment functions such as putenv(). This in turn is because C and POSIX are tightly linked, and POSIX disallows "=" in environment variable names. Python's os.environ, os.getenv(), and os.putenv() are based on the Windows C runtime (i.e. ucrt), so they have the same limits, and probably should since these are C/POSIX constructs. Maybe one day Python will provide native Windows functionality in the os module that directly accesses the environment block of the process and directly exposes the WinAPI GetEnvironmentVariableW and SetEnvironmentVariableW functions. For now, that's only possible with extensions such as PyWin32 or ctypes.

[mattip]

Thanks. So I will keep PyPy's putenv consistent with CPython: python allows =hidden as a name, but then the putenv syscall will fail.