summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSerhiy Storchaka <storchaka@gmail.com>2017-06-25 06:49:15 (GMT)
committerGitHub <noreply@github.com>2017-06-25 06:49:15 (GMT)
commit9c2dc0c58a878ac3d1c44dd0048f8e1cfab2790e (patch)
tree209a2e6a745b9e1aecdca7b62486fa1360a4e70d
parent57ee0c8c9e73c3405a0343e1a05cba673ac7f8cb (diff)
downloadcpython-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.py76
-rw-r--r--Lib/test/test_posix.py15
-rw-r--r--Misc/NEWS3
-rw-r--r--Modules/posixmodule.c32
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'
diff --git a/Misc/NEWS b/Misc/NEWS
index 3f57982..a3a32ae 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -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;
}