Commit 787826c9 authored by Serhiy Storchaka's avatar Serhiy Storchaka Committed by GitHub

[2.7] bpo-30746: Prohibited the '=' character in environment variable names (GH-2382) (#2393)

in `os.putenv()` and `os.spawn*()`..
(cherry picked from commit 77703942)
parent 9dda2cac
...@@ -599,11 +599,32 @@ class URandomFDTests(unittest.TestCase): ...@@ -599,11 +599,32 @@ class URandomFDTests(unittest.TestCase):
assert_python_ok('-c', code) assert_python_ok('-c', code)
class ExecvpeTests(unittest.TestCase): class ExecTests(unittest.TestCase):
def test_execvpe_with_bad_arglist(self): def test_execvpe_with_bad_arglist(self):
self.assertRaises(ValueError, os.execvpe, 'notepad', [], None) 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") @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
class Win32ErrorTests(unittest.TestCase): class Win32ErrorTests(unittest.TestCase):
...@@ -879,6 +900,62 @@ class Win32KillTests(unittest.TestCase): ...@@ -879,6 +900,62 @@ class Win32KillTests(unittest.TestCase):
self._kill_with_event(signal.CTRL_BREAK_EVENT, "CTRL_BREAK_EVENT") 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(): def test_main():
test_support.run_unittest( test_support.run_unittest(
FileTests, FileTests,
...@@ -890,11 +967,12 @@ def test_main(): ...@@ -890,11 +967,12 @@ def test_main():
DevNullTests, DevNullTests,
URandomTests, URandomTests,
URandomFDTests, URandomFDTests,
ExecvpeTests, ExecTests,
Win32ErrorTests, Win32ErrorTests,
TestInvalidFD, TestInvalidFD,
PosixUidGidTests, PosixUidGidTests,
Win32KillTests Win32KillTests,
SpawnTests,
) )
if __name__ == "__main__": if __name__ == "__main__":
......
...@@ -504,6 +504,15 @@ class PosixTester(unittest.TestCase): ...@@ -504,6 +504,15 @@ class PosixTester(unittest.TestCase):
finally: finally:
posix.lchflags(_DUMMY_SYMLINK, dummy_symlink_st.st_flags) 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'), @unittest.skipUnless(hasattr(posix, 'getcwd'),
'test needs posix.getcwd()') 'test needs posix.getcwd()')
def test_getcwd_long_pathnames(self): def test_getcwd_long_pathnames(self):
......
...@@ -52,6 +52,9 @@ Extension Modules ...@@ -52,6 +52,9 @@ Extension Modules
Library 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 - [Security] bpo-30730: Prevent environment variables injection in subprocess on
Windows. Prevent passing other environment variables and command arguments. Windows. Prevent passing other environment variables and command arguments.
......
...@@ -3556,6 +3556,12 @@ posix_spawnve(PyObject *self, PyObject *args) ...@@ -3556,6 +3556,12 @@ posix_spawnve(PyObject *self, PyObject *args)
{ {
goto fail_2; 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; len = PyString_Size(key) + PyString_Size(val) + 2;
p = PyMem_NEW(char, len); p = PyMem_NEW(char, len);
if (p == NULL) { if (p == NULL) {
...@@ -3789,6 +3795,12 @@ posix_spawnvpe(PyObject *self, PyObject *args) ...@@ -3789,6 +3795,12 @@ posix_spawnvpe(PyObject *self, PyObject *args)
{ {
goto fail_2; 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; len = PyString_Size(key) + PyString_Size(val) + 2;
p = PyMem_NEW(char, len); p = PyMem_NEW(char, len);
if (p == NULL) { if (p == NULL) {
...@@ -7185,6 +7197,13 @@ posix_putenv(PyObject *self, PyObject *args) ...@@ -7185,6 +7197,13 @@ posix_putenv(PyObject *self, PyObject *args)
} else { } else {
#endif #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 :-( */ /* XXX This can leak memory -- not easy to fix :-( */
len = strlen(s1) + strlen(s2) + 2; len = strlen(s1) + strlen(s2) + 2;
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment