[3.9] bpo-42146: Fix memory leak in subprocess.Popen() in case of uid/gid overflow...

Fix memory leak in subprocess.Popen() in case of uid/gid overflow

Also add a test that would catch this leak with `--huntrleaks`.

Alas, the test for `extra_groups` also exposes an inconsistency
in our error reporting: we use a custom ValueError for `extra_groups`,
but propagate OverflowError for `user` and `group`.
(cherry picked from commit c0590c0033e86f98cdf5f2ca6898656f98ab4053)

Co-authored-by: Alexey Izbyshev <izbyshev@ispras.ru>
Automerge-Triggered-By: GH:gpshead

diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index ba2844da9add977e964ce4675575b6c7448f0efc..373542463f2e356dbe189d13bdff99d8c5413585 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -1826,6 +1826,10 @@ class POSIXProcessTestCase(BaseTestCase):
         with self.assertRaises(ValueError):
             subprocess.check_call(ZERO_RETURN_CMD, user=-1)
 
+        with self.assertRaises(OverflowError):
+            subprocess.check_call(ZERO_RETURN_CMD,
+                                  cwd=os.curdir, env=os.environ, user=2**64)
+
         if pwd is None and name_uid is not None:
             with self.assertRaises(ValueError):
                 subprocess.check_call(ZERO_RETURN_CMD, user=name_uid)
@@ -1869,6 +1873,10 @@ class POSIXProcessTestCase(BaseTestCase):
         with self.assertRaises(ValueError):
             subprocess.check_call(ZERO_RETURN_CMD, group=-1)
 
+        with self.assertRaises(OverflowError):
+            subprocess.check_call(ZERO_RETURN_CMD,
+                                  cwd=os.curdir, env=os.environ, group=2**64)
+
         if grp is None:
             with self.assertRaises(ValueError):
                 subprocess.check_call(ZERO_RETURN_CMD, group=name_group)
@@ -1917,6 +1925,11 @@ class POSIXProcessTestCase(BaseTestCase):
         with self.assertRaises(ValueError):
             subprocess.check_call(ZERO_RETURN_CMD, extra_groups=[-1])
 
+        with self.assertRaises(ValueError):
+            subprocess.check_call(ZERO_RETURN_CMD,
+                                  cwd=os.curdir, env=os.environ,
+                                  extra_groups=[2**64])
+
         if grp is None:
             with self.assertRaises(ValueError):
                 subprocess.check_call(ZERO_RETURN_CMD,

diff --git a/Misc/NEWS.d/next/Library/2020-10-25-19-25-02.bpo-42146.6A8uvS.rst b/Misc/NEWS.d/next/Library/2020-10-25-19-25-02.bpo-42146.6A8uvS.rst
new file mode 100644 (file)
index 0000000..0418098
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-10-25-19-25-02.bpo-42146.6A8uvS.rst
@@ -0,0 +1,2 @@
+Fix memory leak in :func:`subprocess.Popen` in case an uid (gid) specified in
+`user` (`group`, `extra_groups`) overflows `uid_t` (`gid_t`).

diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 5d1691ace4192003a1449749998e3732fc83fc44..5356417dd7037f4e6ffbe89878d128c027abf015 100644 (file)
--- a/Modules/_posixsubprocess.c
+++ b/Modules/_posixsubprocess.c
@@ -626,7 +626,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
     uid_t uid;
     gid_t gid, *groups = NULL;
     int child_umask;
-    PyObject *cwd_obj, *cwd_obj2;
+    PyObject *cwd_obj, *cwd_obj2 = NULL;
     const char *cwd;
     pid_t pid;
     int need_to_reenable_gc = 0;
@@ -748,7 +748,6 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
         cwd = PyBytes_AsString(cwd_obj2);
     } else {
         cwd = NULL;
-        cwd_obj2 = NULL;
     }
 
     if (groups_list != Py_None) {
@@ -908,6 +907,7 @@ subprocess_fork_exec(PyObject* self, PyObject *args)
     return PyLong_FromPid(pid);
 
 cleanup:
+    Py_XDECREF(cwd_obj2);
     if (envp)
         _Py_FreeCharPArray(envp);
     if (argv)