From: Serhiy Storchaka Date: Sun, 25 Jun 2017 06:50:00 +0000 (+0300) Subject: [2.7] bpo-30746: Prohibited the '=' character in environment variable names (GH-2382... X-Git-Tag: v2.7.14rc1~80 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=787826c9316b03ac8a197078ec1cdf98fa840c5c;p=python [2.7] bpo-30746: Prohibited the '=' character in environment variable names (GH-2382) (#2393) in `os.putenv()` and `os.spawn*()`.. (cherry picked from commit 77703942c5997dff00c48f10df1b29b11645624c) --- diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index b70e285e89..aca03fdabb 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -599,11 +599,32 @@ class URandomFDTests(unittest.TestCase): assert_python_ok('-c', code) -class ExecvpeTests(unittest.TestCase): +class ExecTests(unittest.TestCase): def test_execvpe_with_bad_arglist(self): self.assertRaises(ValueError, os.execvpe, 'notepad', [], None) + 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(TypeError): + 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(TypeError): + 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): @@ -879,6 +900,62 @@ class Win32KillTests(unittest.TestCase): self._kill_with_event(signal.CTRL_BREAK_EVENT, "CTRL_BREAK_EVENT") +class SpawnTests(unittest.TestCase): + 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 TypeError: + 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 TypeError: + 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 = test_support.TESTFN + self.addCleanup(test_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) + + @unittest.skipUnless(hasattr(os, 'spawnve'), 'test needs os.spawnve()') + def test_spawnve_invalid_env(self): + self._test_invalid_env(os.spawnve) + + @unittest.skipUnless(hasattr(os, 'spawnvpe'), 'test needs os.spawnvpe()') + def test_spawnvpe_invalid_env(self): + self._test_invalid_env(os.spawnvpe) + + def test_main(): test_support.run_unittest( FileTests, @@ -890,11 +967,12 @@ def test_main(): DevNullTests, URandomTests, URandomFDTests, - ExecvpeTests, + ExecTests, Win32ErrorTests, TestInvalidFD, PosixUidGidTests, - Win32KillTests + Win32KillTests, + SpawnTests, ) if __name__ == "__main__": diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py index e0c84f2d6c..f1626b717f 100644 --- a/Lib/test/test_posix.py +++ b/Lib/test/test_posix.py @@ -504,6 +504,15 @@ class PosixTester(unittest.TestCase): finally: posix.lchflags(_DUMMY_SYMLINK, dummy_symlink_st.st_flags) + @unittest.skipUnless(hasattr(os, "putenv"), "requires os.putenv()") + def test_putenv(self): + with self.assertRaises(TypeError): + os.putenv('FRUIT\0VEGETABLE', 'cabbage') + with self.assertRaises(TypeError): + os.putenv('FRUIT', 'orange\0VEGETABLE=cabbage') + with self.assertRaises(ValueError): + os.putenv('FRUIT=ORANGE', 'lemon') + @unittest.skipUnless(hasattr(posix, 'getcwd'), 'test needs posix.getcwd()') def test_getcwd_long_pathnames(self): diff --git a/Misc/NEWS b/Misc/NEWS index 564c8df992..94af58815f 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -52,6 +52,9 @@ Extension Modules 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 ffeb715c9f..a06c56e069 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3556,6 +3556,12 @@ posix_spawnve(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; + } len = PyString_Size(key) + PyString_Size(val) + 2; p = PyMem_NEW(char, len); if (p == NULL) { @@ -3789,6 +3795,12 @@ posix_spawnvpe(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; + } len = PyString_Size(key) + PyString_Size(val) + 2; p = PyMem_NEW(char, len); if (p == NULL) { @@ -7185,6 +7197,13 @@ posix_putenv(PyObject *self, PyObject *args) } else { #endif + /* Search from index 1 because on Windows starting '=' is allowed for + defining hidden environment variables. */ + if (*s1 == '\0' || strchr(s1 + 1, '=') != NULL) { + PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); + return NULL; + } + /* XXX This can leak memory -- not easy to fix :-( */ len = strlen(s1) + strlen(s2) + 2; #ifdef MS_WINDOWS