diff options
author | Serhiy Storchaka <storchaka@gmail.com> | 2017-06-25 06:49:15 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-06-25 06:49:15 (GMT) |
commit | 9c2dc0c58a878ac3d1c44dd0048f8e1cfab2790e (patch) | |
tree | 209a2e6a745b9e1aecdca7b62486fa1360a4e70d | |
parent | 57ee0c8c9e73c3405a0343e1a05cba673ac7f8cb (diff) | |
download | cpython-9c2dc0c58a878ac3d1c44dd0048f8e1cfab2790e.zip cpython-9c2dc0c58a878ac3d1c44dd0048f8e1cfab2790e.tar.gz cpython-9c2dc0c58a878ac3d1c44dd0048f8e1cfab2790e.tar.bz2 |
[3.6] bpo-30746: Prohibited the '=' character in environment variable names (GH-2382) (#2391)
in `os.putenv()` and `os.spawn*()`..
(cherry picked from commit 77703942c5997dff00c48f10df1b29b11645624c)
-rw-r--r-- | Lib/test/test_os.py | 76 | ||||
-rw-r--r-- | Lib/test/test_posix.py | 15 | ||||
-rw-r--r-- | Misc/NEWS | 3 | ||||
-rw-r--r-- | Modules/posixmodule.c | 32 |
4 files changed, 122 insertions, 4 deletions
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 8612ec9..fdeb9a9 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -1553,6 +1553,27 @@ class ExecTests(unittest.TestCase): if os.name != "nt": self._test_internal_execvpe(bytes) + def test_execve_invalid_env(self): + args = [sys.executable, '-c', 'pass'] + + # null character in the enviroment variable name + newenv = os.environ.copy() + newenv["FRUIT\0VEGETABLE"] = "cabbage" + with self.assertRaises(ValueError): + os.execve(args[0], args, newenv) + + # null character in the enviroment variable value + newenv = os.environ.copy() + newenv["FRUIT"] = "orange\0VEGETABLE=cabbage" + with self.assertRaises(ValueError): + os.execve(args[0], args, newenv) + + # equal character in the enviroment variable name + newenv = os.environ.copy() + newenv["FRUIT=ORANGE"] = "lemon" + with self.assertRaises(ValueError): + os.execve(args[0], args, newenv) + @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests") class Win32ErrorTests(unittest.TestCase): @@ -2364,6 +2385,61 @@ class SpawnTests(unittest.TestCase): self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, args[0], ('',), {}) self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, args[0], [''], {}) + def _test_invalid_env(self, spawn): + args = [sys.executable, '-c', 'pass'] + + # null character in the enviroment variable name + newenv = os.environ.copy() + newenv["FRUIT\0VEGETABLE"] = "cabbage" + try: + exitcode = spawn(os.P_WAIT, args[0], args, newenv) + except ValueError: + pass + else: + self.assertEqual(exitcode, 127) + + # null character in the enviroment variable value + newenv = os.environ.copy() + newenv["FRUIT"] = "orange\0VEGETABLE=cabbage" + try: + exitcode = spawn(os.P_WAIT, args[0], args, newenv) + except ValueError: + pass + else: + self.assertEqual(exitcode, 127) + + # equal character in the enviroment variable name + newenv = os.environ.copy() + newenv["FRUIT=ORANGE"] = "lemon" + try: + exitcode = spawn(os.P_WAIT, args[0], args, newenv) + except ValueError: + pass + else: + self.assertEqual(exitcode, 127) + + # equal character in the enviroment variable value + filename = support.TESTFN + self.addCleanup(support.unlink, filename) + with open(filename, "w") as fp: + fp.write('import sys, os\n' + 'if os.getenv("FRUIT") != "orange=lemon":\n' + ' raise AssertionError') + args = [sys.executable, filename] + newenv = os.environ.copy() + newenv["FRUIT"] = "orange=lemon" + exitcode = spawn(os.P_WAIT, args[0], args, newenv) + self.assertEqual(exitcode, 0) + + @requires_os_func('spawnve') + def test_spawnve_invalid_env(self): + self._test_invalid_env(os.spawnve) + + @requires_os_func('spawnvpe') + def test_spawnvpe_invalid_env(self): + self._test_invalid_env(os.spawnvpe) + + # The introduction of this TestCase caused at least two different errors on # *nix buildbots. Temporarily skip this to let the buildbots move along. @unittest.skip("Skip due to platform/environment differences on *NIX buildbots") diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index 029d081..4ca6230 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -751,6 +751,21 @@ class PosixTester(unittest.TestCase): self.assertEqual(type(k), item_type) self.assertEqual(type(v), item_type) + @unittest.skipUnless(hasattr(os, "putenv"), "requires os.putenv()") + def test_putenv(self): + with self.assertRaises(ValueError): + os.putenv('FRUIT\0VEGETABLE', 'cabbage') + with self.assertRaises(ValueError): + os.putenv(b'FRUIT\0VEGETABLE', b'cabbage') + with self.assertRaises(ValueError): + os.putenv('FRUIT', 'orange\0VEGETABLE=cabbage') + with self.assertRaises(ValueError): + os.putenv(b'FRUIT', b'orange\0VEGETABLE=cabbage') + with self.assertRaises(ValueError): + os.putenv('FRUIT=ORANGE', 'lemon') + with self.assertRaises(ValueError): + os.putenv(b'FRUIT=ORANGE', b'lemon') + @unittest.skipUnless(hasattr(posix, 'getcwd'), 'test needs posix.getcwd()') def test_getcwd_long_pathnames(self): dirname = 'getcwd-test-directory-0123456789abcdef-01234567890abcdef' @@ -13,6 +13,9 @@ Core and Builtins Library ------- +- bpo-30746: Prohibited the '=' character in environment variable names in + ``os.putenv()`` and ``os.spawn*()``. + - [Security] bpo-30730: Prevent environment variables injection in subprocess on Windows. Prevent passing other environment variables and command arguments. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 11aaeef..0337890 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -4817,6 +4817,14 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) Py_DECREF(key2); goto error; } + /* Search from index 1 because on Windows starting '=' is allowed for + defining hidden environment variables. */ + if (PyUnicode_GET_LENGTH(key2) == 0 || + PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1) + { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + goto error; + } keyval = PyUnicode_FromFormat("%U=%U", key2, val2); #else if (!PyUnicode_FSConverter(key, &key2)) @@ -4825,6 +4833,12 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) Py_DECREF(key2); goto error; } + if (PyBytes_GET_SIZE(key2) == 0 || + strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL) + { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + goto error; + } keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2), PyBytes_AS_STRING(val2)); #endif @@ -8864,9 +8878,16 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) { const wchar_t *env; + /* Search from index 1 because on Windows starting '=' is allowed for + defining hidden environment variables. */ + if (PyUnicode_GET_LENGTH(name) == 0 || + PyUnicode_FindChar(name, '=', 1, PyUnicode_GET_LENGTH(name), 1) != -1) + { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + return NULL; + } PyObject *unicode = PyUnicode_FromFormat("%U=%U", name, value); if (unicode == NULL) { - PyErr_NoMemory(); return NULL; } if (_MAX_ENV < PyUnicode_GET_LENGTH(unicode)) { @@ -8908,12 +8929,15 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) { PyObject *bytes = NULL; char *env; - const char *name_string = PyBytes_AsString(name); - const char *value_string = PyBytes_AsString(value); + const char *name_string = PyBytes_AS_STRING(name); + const char *value_string = PyBytes_AS_STRING(value); + if (strchr(name_string, '=') != NULL) { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + return NULL; + } bytes = PyBytes_FromFormat("%s=%s", name_string, value_string); if (bytes == NULL) { - PyErr_NoMemory(); return NULL; } |