From d6cf4261e655eefbc31213ca577c3a9cc9043735 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 9 Jun 2023 23:00:26 +0200 Subject: [PATCH 1/3] gh-105605: Harden pyexpat initialisation Add proper error handling to add_errors_module() to prevent exceptions from possibly being overwritten, and objects from being decref'ed twice. --- ...23-06-09-23-00-13.gh-issue-105605.YuwqxY.rst | 3 +++ Modules/pyexpat.c | 17 ++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-06-09-23-00-13.gh-issue-105605.YuwqxY.rst diff --git a/Misc/NEWS.d/next/Library/2023-06-09-23-00-13.gh-issue-105605.YuwqxY.rst b/Misc/NEWS.d/next/Library/2023-06-09-23-00-13.gh-issue-105605.YuwqxY.rst new file mode 100644 index 00000000000000..5fba6d293a785e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-06-09-23-00-13.gh-issue-105605.YuwqxY.rst @@ -0,0 +1,3 @@ +Harden :mod:`pyexpat` error handling during module initialisation to prevent +exceptions from possibly being overwritten, and objects from being +dereferenced twice. diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index 27f2d0a6a0f68d..e85a88d19fcc02 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1781,8 +1781,11 @@ add_errors_module(PyObject *mod) } PyObject *codes_dict = PyDict_New(); + if (codes_dict == NULL) { + return -1; + } PyObject *rev_codes_dict = PyDict_New(); - if (codes_dict == NULL || rev_codes_dict == NULL) { + if (rev_codes_dict == NULL) { goto error; } @@ -1803,17 +1806,17 @@ add_errors_module(PyObject *mod) goto error; } - if (PyModule_AddObject(errors_module, "codes", Py_NewRef(codes_dict)) < 0) { - Py_DECREF(codes_dict); + int rc = PyModule_AddObjectRef(errors_module, "codes", codes_dict); + Py_CLEAR(codes_dict); + if (rc < 0) { goto error; } - Py_CLEAR(codes_dict); - if (PyModule_AddObject(errors_module, "messages", Py_NewRef(rev_codes_dict)) < 0) { - Py_DECREF(rev_codes_dict); + rc = PyModule_AddObject(errors_module, "messages", rev_codes_dict); + Py_CLEAR(rev_codes_dict); + if (rc < 0) { goto error; } - Py_CLEAR(rev_codes_dict); return 0; From 143c3e371af814c560dca7d9fcef946ff0513626 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Fri, 9 Jun 2023 23:16:15 +0200 Subject: [PATCH 2/3] => PyModule_AddObjectREF --- Modules/pyexpat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index e85a88d19fcc02..a57216083c2ffe 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1812,7 +1812,7 @@ add_errors_module(PyObject *mod) goto error; } - rc = PyModule_AddObject(errors_module, "messages", rev_codes_dict); + rc = PyModule_AddObjectRef(errors_module, "messages", rev_codes_dict); Py_CLEAR(rev_codes_dict); if (rc < 0) { goto error; From 5fd45cddd3c9a600640f258e8c94debf9a2f0a69 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Sun, 11 Jun 2023 21:46:35 +0200 Subject: [PATCH 3/3] Add comment about borrowed ref --- Modules/pyexpat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c index a57216083c2ffe..e3333fff00b2b2 100644 --- a/Modules/pyexpat.c +++ b/Modules/pyexpat.c @@ -1775,6 +1775,7 @@ add_error(PyObject *errors_module, PyObject *codes_dict, static int add_errors_module(PyObject *mod) { + // add_submodule() returns a borrowed ref. PyObject *errors_module = add_submodule(mod, MODULE_NAME ".errors"); if (errors_module == NULL) { return -1;