[2.7] bpo-30730: Prevent environment variables injection in subprocess on Windows. (GH-2325) (#2372)
Prevent passing other invalid environment variables and command arguments..
(cherry picked from commit d174d24a5d37d1516b885dc7c82f71ecd5930700)
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 6272758..5220891 100644
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -385,6 +385,46 @@
self.addCleanup(p.stdout.close)
self.assertEqual(p.stdout.read(), "orange")
+ def test_invalid_cmd(self):
+ # null character in the command name
+ cmd = sys.executable + '\0'
+ with self.assertRaises(TypeError):
+ subprocess.Popen([cmd, "-c", "pass"])
+
+ # null character in the command argument
+ with self.assertRaises(TypeError):
+ subprocess.Popen([sys.executable, "-c", "pass#\0"])
+
+ def test_invalid_env(self):
+ # null character in the enviroment variable name
+ newenv = os.environ.copy()
+ newenv["FRUIT\0VEGETABLE"] = "cabbage"
+ with self.assertRaises(TypeError):
+ subprocess.Popen([sys.executable, "-c", "pass"], env=newenv)
+
+ # null character in the enviroment variable value
+ newenv = os.environ.copy()
+ newenv["FRUIT"] = "orange\0VEGETABLE=cabbage"
+ with self.assertRaises(TypeError):
+ subprocess.Popen([sys.executable, "-c", "pass"], env=newenv)
+
+ # equal character in the enviroment variable name
+ newenv = os.environ.copy()
+ newenv["FRUIT=ORANGE"] = "lemon"
+ with self.assertRaises(ValueError):
+ subprocess.Popen([sys.executable, "-c", "pass"], env=newenv)
+
+ # equal character in the enviroment variable value
+ newenv = os.environ.copy()
+ newenv["FRUIT"] = "orange=lemon"
+ p = subprocess.Popen([sys.executable, "-c",
+ 'import sys, os;'
+ 'sys.stdout.write(os.getenv("FRUIT"))'],
+ stdout=subprocess.PIPE,
+ env=newenv)
+ stdout, stderr = p.communicate()
+ self.assertEqual(stdout, "orange=lemon")
+
def test_communicate_stdin(self):
p = subprocess.Popen([sys.executable, "-c",
'import sys;'
diff --git a/Misc/NEWS b/Misc/NEWS
index 30e3877..564c8df 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -52,6 +52,9 @@
Library
-------
+- [Security] bpo-30730: Prevent environment variables injection in subprocess on
+ Windows. Prevent passing other environment variables and command arguments.
+
- [Security] bpo-30694: Upgrade expat copy from 2.2.0 to 2.2.1 to get fixes
of multiple security vulnerabilities including: CVE-2017-9233 (External
entity infinite loop DoS), CVE-2016-9063 (Integer overflow, re-fix),
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index ad50536..ffeb715 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -3315,6 +3315,12 @@
{
goto fail_2;
}
+ /* Search from index 1 because on Windows starting '=' is allowed for
+ defining hidden environment variables. */
+ if (*k == '\0' || strchr(k + 1, '=') != NULL) {
+ PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
+ goto fail_2;
+ }
#if defined(PYOS_OS2)
/* Omit Pseudo-Env Vars that Would Confuse Programs if Passed On */
diff --git a/PC/_subprocess.c b/PC/_subprocess.c
index ffe8f41..f73d14f 100644
--- a/PC/_subprocess.c
+++ b/PC/_subprocess.c
@@ -352,7 +352,7 @@
p = PyString_AS_STRING(out);
for (i = 0; i < envsize; i++) {
- int ksize, vsize, totalsize;
+ size_t ksize, vsize, totalsize;
PyObject* key = PyList_GET_ITEM(keys, i);
PyObject* value = PyList_GET_ITEM(values, i);
@@ -363,10 +363,22 @@
}
ksize = PyString_GET_SIZE(key);
vsize = PyString_GET_SIZE(value);
+ if (strlen(PyString_AS_STRING(key)) != ksize ||
+ strlen(PyString_AS_STRING(value)) != vsize)
+ {
+ PyErr_SetString(PyExc_TypeError, "embedded null character");
+ goto error;
+ }
+ /* Search from index 1 because on Windows starting '=' is allowed for
+ defining hidden environment variables. */
+ if (ksize == 0 || strchr(PyString_AS_STRING(key) + 1, '=') != NULL) {
+ PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
+ goto error;
+ }
totalsize = (p - PyString_AS_STRING(out)) + ksize + 1 +
vsize + 1 + 1;
if (totalsize > PyString_GET_SIZE(out)) {
- int offset = p - PyString_AS_STRING(out);
+ size_t offset = p - PyString_AS_STRING(out);
if (_PyString_Resize(&out, totalsize + 1024))
goto exit;
p = PyString_AS_STRING(out) + offset;