diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 627275891fb3b2..5220891cbb2ac5 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -385,6 +385,46 @@ def test_env(self): 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 30e3877718decc..564c8df9923fc3 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -52,6 +52,9 @@ Extension Modules 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 ad505364bda9da..ffeb715c9f156e 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3315,6 +3315,12 @@ posix_execve(PyObject *self, PyObject *args) { 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 ffe8f41d573df0..f73d14f57930f2 100644 --- a/PC/_subprocess.c +++ b/PC/_subprocess.c @@ -352,7 +352,7 @@ getenvironment(PyObject* environment) 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 @@ getenvironment(PyObject* environment) } 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;