python · serhiy-storchaka · Jun 25, 2017 · Jun 25, 2017 · Jun 25, 2017 · Jun 25, 2017
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
@@ -1501,6 +1501,27 @@ def test_internal_execvpe_str(self):
        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):
@@ -2184,6 +2205,62 @@ def test_waitpid(self):
        self.assertEqual(status, (pid, 0))


+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 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)
+
+    @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)
+
+
 # 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
@@ -747,6 +747,21 @@ def test_environ(self):
            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'

diff --git a/Misc/NEWS b/Misc/NEWS
@@ -59,6 +59,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
@@ -4964,8 +4964,14 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
            goto error;
        }

-        k = PyBytes_AsString(key2);
-        v = PyBytes_AsString(val2);
+        k = PyBytes_AS_STRING(key2);
+        v = PyBytes_AS_STRING(val2);
+        /* 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 error;
+        }
        len = PyBytes_GET_SIZE(key2) + PyBytes_GET_SIZE(val2) + 2;

        p = PyMem_NEW(char, len);
@@ -9069,9 +9075,16 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value)
 {
    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)) {
@@ -9113,12 +9126,15 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value)
 {
    PyObject *bytes = NULL;
    char *env;
-    char *name_string = PyBytes_AsString(name);
-    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;
    }