gh-139748: fix leaks in AC error paths when using unicode FS-based converters (#139765)

This commit is contained in:
Bénédikt Tran 2025-10-08 17:22:44 +02:00 committed by GitHub
parent 570d17259f
commit b04a57deef
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 88 additions and 46 deletions

View file

@ -2980,6 +2980,8 @@ def test_cli_converters(self):
"uint64", "uint64",
"uint8", "uint8",
"unicode", "unicode",
"unicode_fs_decoded",
"unicode_fs_encoded",
"unsigned_char", "unsigned_char",
"unsigned_int", "unsigned_int",
"unsigned_long", "unsigned_long",

View file

@ -651,6 +651,21 @@ def test_compile_filename(self):
compile('pass', filename, 'exec') compile('pass', filename, 'exec')
self.assertRaises(TypeError, compile, 'pass', list(b'file.py'), 'exec') self.assertRaises(TypeError, compile, 'pass', list(b'file.py'), 'exec')
def test_compile_filename_refleak(self):
# Regression tests for reference leak in PyUnicode_FSDecoder.
# See https://github.com/python/cpython/issues/139748.
mortal_str = 'this is a mortal string'
# check error path when 'mode' AC conversion failed
self.assertRaises(TypeError, compile, b'', mortal_str, mode=1234)
# check error path when 'optimize' AC conversion failed
self.assertRaises(OverflowError, compile, b'', mortal_str,
'exec', optimize=1 << 1000)
# check error path when 'dont_inherit' AC conversion failed
class EvilBool:
def __bool__(self): raise ValueError
self.assertRaises(ValueError, compile, b'', mortal_str,
'exec', dont_inherit=EvilBool())
@support.cpython_only @support.cpython_only
def test_same_filename_used(self): def test_same_filename_used(self):
s = """def f(): pass\ndef g(): pass""" s = """def f(): pass\ndef g(): pass"""

View file

@ -579,6 +579,13 @@ def test_nested_genexpr(self):
self.assertEqual(sorted(st.get_identifiers()), [".0", "y"]) self.assertEqual(sorted(st.get_identifiers()), [".0", "y"])
self.assertEqual(st.get_children(), []) self.assertEqual(st.get_children(), [])
def test__symtable_refleak(self):
# Regression test for reference leak in PyUnicode_FSDecoder.
# See https://github.com/python/cpython/issues/139748.
mortal_str = 'this is a mortal string'
# check error path when 'compile_type' AC conversion failed
self.assertRaises(TypeError, symtable.symtable, '', mortal_str, 1)
class ComprehensionTests(unittest.TestCase): class ComprehensionTests(unittest.TestCase):
def get_identifiers_recursive(self, st, res): def get_identifiers_recursive(self, st, res):

View file

@ -0,0 +1,2 @@
Fix reference leaks in error branches of functions accepting path strings or
bytes such as :func:`compile` and :func:`os.system`. Patch by Bénédikt Tran.

View file

@ -1858,14 +1858,14 @@ _certificate_to_der(_sslmodulestate *state, X509 *certificate)
/*[clinic input] /*[clinic input]
_ssl._test_decode_cert _ssl._test_decode_cert
path: object(converter="PyUnicode_FSConverter") path: unicode_fs_encoded
/ /
[clinic start generated code]*/ [clinic start generated code]*/
static PyObject * static PyObject *
_ssl__test_decode_cert_impl(PyObject *module, PyObject *path) _ssl__test_decode_cert_impl(PyObject *module, PyObject *path)
/*[clinic end generated code: output=96becb9abb23c091 input=cdeaaf02d4346628]*/ /*[clinic end generated code: output=96becb9abb23c091 input=cb4988d5e651a4f8]*/
{ {
PyObject *retval = NULL; PyObject *retval = NULL;
X509 *x=NULL; X509 *x=NULL;
@ -1895,7 +1895,6 @@ _ssl__test_decode_cert_impl(PyObject *module, PyObject *path)
X509_free(x); X509_free(x);
fail0: fail0:
Py_DECREF(path);
if (cert != NULL) BIO_free(cert); if (cert != NULL) BIO_free(cert);
return retval; return retval;
} }

View file

@ -48,7 +48,7 @@ static PyObject *
_ssl__test_decode_cert(PyObject *module, PyObject *arg) _ssl__test_decode_cert(PyObject *module, PyObject *arg)
{ {
PyObject *return_value = NULL; PyObject *return_value = NULL;
PyObject *path; PyObject *path = NULL;
if (!PyUnicode_FSConverter(arg, &path)) { if (!PyUnicode_FSConverter(arg, &path)) {
goto exit; goto exit;
@ -56,6 +56,9 @@ _ssl__test_decode_cert(PyObject *module, PyObject *arg)
return_value = _ssl__test_decode_cert_impl(module, path); return_value = _ssl__test_decode_cert_impl(module, path);
exit: exit:
/* Cleanup for path */
Py_XDECREF(path);
return return_value; return return_value;
} }
@ -3322,4 +3325,4 @@ exit:
#ifndef _SSL_ENUM_CRLS_METHODDEF #ifndef _SSL_ENUM_CRLS_METHODDEF
#define _SSL_ENUM_CRLS_METHODDEF #define _SSL_ENUM_CRLS_METHODDEF
#endif /* !defined(_SSL_ENUM_CRLS_METHODDEF) */ #endif /* !defined(_SSL_ENUM_CRLS_METHODDEF) */
/*[clinic end generated code: output=5a630a1e83927d47 input=a9049054013a1b77]*/ /*[clinic end generated code: output=3b6c9cbfc4660ecb input=a9049054013a1b77]*/

View file

@ -479,7 +479,7 @@ static PyObject *
_socket_if_nametoindex(PyObject *module, PyObject *arg) _socket_if_nametoindex(PyObject *module, PyObject *arg)
{ {
PyObject *return_value = NULL; PyObject *return_value = NULL;
PyObject *oname; PyObject *oname = NULL;
if (!PyUnicode_FSConverter(arg, &oname)) { if (!PyUnicode_FSConverter(arg, &oname)) {
goto exit; goto exit;
@ -487,6 +487,9 @@ _socket_if_nametoindex(PyObject *module, PyObject *arg)
return_value = _socket_if_nametoindex_impl(module, oname); return_value = _socket_if_nametoindex_impl(module, oname);
exit: exit:
/* Cleanup for oname */
Py_XDECREF(oname);
return return_value; return return_value;
} }
@ -538,4 +541,4 @@ exit:
#ifndef _SOCKET_IF_INDEXTONAME_METHODDEF #ifndef _SOCKET_IF_INDEXTONAME_METHODDEF
#define _SOCKET_IF_INDEXTONAME_METHODDEF #define _SOCKET_IF_INDEXTONAME_METHODDEF
#endif /* !defined(_SOCKET_IF_INDEXTONAME_METHODDEF) */ #endif /* !defined(_SOCKET_IF_INDEXTONAME_METHODDEF) */
/*[clinic end generated code: output=0376c46b76ae2bce input=a9049054013a1b77]*/ /*[clinic end generated code: output=36051ebf6ad1e6f8 input=a9049054013a1b77]*/

View file

@ -22,7 +22,7 @@ _symtable_symtable(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
{ {
PyObject *return_value = NULL; PyObject *return_value = NULL;
PyObject *source; PyObject *source;
PyObject *filename; PyObject *filename = NULL;
const char *startstr; const char *startstr;
if (!_PyArg_CheckPositional("symtable", nargs, 3, 3)) { if (!_PyArg_CheckPositional("symtable", nargs, 3, 3)) {
@ -48,6 +48,9 @@ _symtable_symtable(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
return_value = _symtable_symtable_impl(module, source, filename, startstr); return_value = _symtable_symtable_impl(module, source, filename, startstr);
exit: exit:
/* Cleanup for filename */
Py_XDECREF(filename);
return return_value; return return_value;
} }
/*[clinic end generated code: output=931964a76a72f850 input=a9049054013a1b77]*/ /*[clinic end generated code: output=7a8545d9a1efe837 input=a9049054013a1b77]*/

View file

@ -3137,17 +3137,6 @@ class dev_t_return_converter(unsigned_long_return_converter):
conversion_fn = '_PyLong_FromDev' conversion_fn = '_PyLong_FromDev'
unsigned_cast = '(dev_t)' unsigned_cast = '(dev_t)'
class FSConverter_converter(CConverter):
type = 'PyObject *'
converter = 'PyUnicode_FSConverter'
def converter_init(self):
if self.default is not unspecified:
fail("FSConverter_converter does not support default values")
self.c_default = 'NULL'
def cleanup(self):
return "Py_XDECREF(" + self.name + ");\n"
class pid_t_converter(CConverter): class pid_t_converter(CConverter):
type = 'pid_t' type = 'pid_t'
format_unit = '" _Py_PARSE_PID "' format_unit = '" _Py_PARSE_PID "'
@ -3211,7 +3200,7 @@ class confname_converter(CConverter):
""", argname=argname, converter=self.converter, table=self.table) """, argname=argname, converter=self.converter, table=self.table)
[python start generated code]*/ [python start generated code]*/
/*[python end generated code: output=da39a3ee5e6b4b0d input=8189d5ae78244626]*/ /*[python end generated code: output=da39a3ee5e6b4b0d input=d2759f2332cd39b3]*/
/*[clinic input] /*[clinic input]
@ -6135,14 +6124,14 @@ os_system_impl(PyObject *module, const wchar_t *command)
/*[clinic input] /*[clinic input]
os.system -> long os.system -> long
command: FSConverter command: unicode_fs_encoded
Execute the command in a subshell. Execute the command in a subshell.
[clinic start generated code]*/ [clinic start generated code]*/
static long static long
os_system_impl(PyObject *module, PyObject *command) os_system_impl(PyObject *module, PyObject *command)
/*[clinic end generated code: output=290fc437dd4f33a0 input=86a58554ba6094af]*/ /*[clinic end generated code: output=290fc437dd4f33a0 input=47c6f24b6dc92881]*/
{ {
long result; long result;
const char *bytes = PyBytes_AsString(command); const char *bytes = PyBytes_AsString(command);
@ -9328,7 +9317,7 @@ os_getgroups_impl(PyObject *module)
/*[clinic input] /*[clinic input]
os.initgroups os.initgroups
username as oname: FSConverter username as oname: unicode_fs_encoded
gid: int gid: int
/ /
@ -9341,12 +9330,12 @@ group id.
static PyObject * static PyObject *
os_initgroups_impl(PyObject *module, PyObject *oname, int gid) os_initgroups_impl(PyObject *module, PyObject *oname, int gid)
/*[clinic end generated code: output=7f074d30a425fd3a input=df3d54331b0af204]*/ /*[clinic end generated code: output=7f074d30a425fd3a input=984e60c7fed88cb4]*/
#else #else
/*[clinic input] /*[clinic input]
os.initgroups os.initgroups
username as oname: FSConverter username as oname: unicode_fs_encoded
gid: gid_t gid: gid_t
/ /
@ -9359,7 +9348,7 @@ group id.
static PyObject * static PyObject *
os_initgroups_impl(PyObject *module, PyObject *oname, gid_t gid) os_initgroups_impl(PyObject *module, PyObject *oname, gid_t gid)
/*[clinic end generated code: output=59341244521a9e3f input=0cb91bdc59a4c564]*/ /*[clinic end generated code: output=59341244521a9e3f input=17d8fbe2dea42ca4]*/
#endif #endif
{ {
const char *username = PyBytes_AS_STRING(oname); const char *username = PyBytes_AS_STRING(oname);
@ -13115,8 +13104,8 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value)
/*[clinic input] /*[clinic input]
os.putenv os.putenv
name: FSConverter name: unicode_fs_encoded
value: FSConverter value: unicode_fs_encoded
/ /
Change or add an environment variable. Change or add an environment variable.
@ -13124,7 +13113,7 @@ Change or add an environment variable.
static PyObject * static PyObject *
os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) os_putenv_impl(PyObject *module, PyObject *name, PyObject *value)
/*[clinic end generated code: output=d29a567d6b2327d2 input=a97bc6152f688d31]*/ /*[clinic end generated code: output=d29a567d6b2327d2 input=84fcd30f873c8c45]*/
{ {
const char *name_string = PyBytes_AS_STRING(name); const char *name_string = PyBytes_AS_STRING(name);
const char *value_string = PyBytes_AS_STRING(value); const char *value_string = PyBytes_AS_STRING(value);
@ -13167,7 +13156,7 @@ os_unsetenv_impl(PyObject *module, PyObject *name)
#else #else
/*[clinic input] /*[clinic input]
os.unsetenv os.unsetenv
name: FSConverter name: unicode_fs_encoded
/ /
Delete an environment variable. Delete an environment variable.
@ -13175,7 +13164,7 @@ Delete an environment variable.
static PyObject * static PyObject *
os_unsetenv_impl(PyObject *module, PyObject *name) os_unsetenv_impl(PyObject *module, PyObject *name)
/*[clinic end generated code: output=54c4137ab1834f02 input=2bb5288a599c7107]*/ /*[clinic end generated code: output=54c4137ab1834f02 input=78ff12e505ade80a]*/
{ {
if (PySys_Audit("os.unsetenv", "(O)", name) < 0) { if (PySys_Audit("os.unsetenv", "(O)", name) < 0) {
return NULL; return NULL;
@ -15229,14 +15218,14 @@ os_urandom_impl(PyObject *module, Py_ssize_t size)
/*[clinic input] /*[clinic input]
os.memfd_create os.memfd_create
name: FSConverter name: unicode_fs_encoded
flags: unsigned_int(bitwise=True, c_default="MFD_CLOEXEC") = MFD_CLOEXEC flags: unsigned_int(bitwise=True, c_default="MFD_CLOEXEC") = MFD_CLOEXEC
[clinic start generated code]*/ [clinic start generated code]*/
static PyObject * static PyObject *
os_memfd_create_impl(PyObject *module, PyObject *name, unsigned int flags) os_memfd_create_impl(PyObject *module, PyObject *name, unsigned int flags)
/*[clinic end generated code: output=6681ede983bdb9a6 input=a42cfc199bcd56e9]*/ /*[clinic end generated code: output=6681ede983bdb9a6 input=cd0eb092cfac474b]*/
{ {
int fd; int fd;
const char *bytes = PyBytes_AS_STRING(name); const char *bytes = PyBytes_AS_STRING(name);

View file

@ -7278,7 +7278,7 @@ Returns a list of network interface information (index, name) tuples.");
/*[clinic input] /*[clinic input]
_socket.if_nametoindex _socket.if_nametoindex
oname: object(converter="PyUnicode_FSConverter") oname: unicode_fs_encoded
/ /
Returns the interface index corresponding to the interface name if_name. Returns the interface index corresponding to the interface name if_name.
@ -7286,7 +7286,7 @@ Returns the interface index corresponding to the interface name if_name.
static PyObject * static PyObject *
_socket_if_nametoindex_impl(PyObject *module, PyObject *oname) _socket_if_nametoindex_impl(PyObject *module, PyObject *oname)
/*[clinic end generated code: output=289a411614f30244 input=01e0f1205307fb77]*/ /*[clinic end generated code: output=289a411614f30244 input=6125dc20683560cf]*/
{ {
#ifdef MS_WINDOWS #ifdef MS_WINDOWS
NET_IFINDEX index; NET_IFINDEX index;
@ -7295,7 +7295,6 @@ _socket_if_nametoindex_impl(PyObject *module, PyObject *oname)
#endif #endif
index = if_nametoindex(PyBytes_AS_STRING(oname)); index = if_nametoindex(PyBytes_AS_STRING(oname));
Py_DECREF(oname);
if (index == 0) { if (index == 0) {
/* if_nametoindex() doesn't set errno */ /* if_nametoindex() doesn't set errno */
PyErr_SetString(PyExc_OSError, "no interface with this name"); PyErr_SetString(PyExc_OSError, "no interface with this name");

View file

@ -13,7 +13,7 @@ module _symtable
_symtable.symtable _symtable.symtable
source: object source: object
filename: object(converter='PyUnicode_FSDecoder') filename: unicode_fs_decoded
startstr: str startstr: str
/ /
@ -23,7 +23,7 @@ Return symbol and scope dictionaries used internally by compiler.
static PyObject * static PyObject *
_symtable_symtable_impl(PyObject *module, PyObject *source, _symtable_symtable_impl(PyObject *module, PyObject *source,
PyObject *filename, const char *startstr) PyObject *filename, const char *startstr)
/*[clinic end generated code: output=59eb0d5fc7285ac4 input=9dd8a50c0c36a4d7]*/ /*[clinic end generated code: output=59eb0d5fc7285ac4 input=436ffff90d02e4f6]*/
{ {
struct symtable *st; struct symtable *st;
PyObject *t; PyObject *t;
@ -47,12 +47,10 @@ _symtable_symtable_impl(PyObject *module, PyObject *source,
else { else {
PyErr_SetString(PyExc_ValueError, PyErr_SetString(PyExc_ValueError,
"symtable() arg 3 must be 'exec' or 'eval' or 'single'"); "symtable() arg 3 must be 'exec' or 'eval' or 'single'");
Py_DECREF(filename);
Py_XDECREF(source_copy); Py_XDECREF(source_copy);
return NULL; return NULL;
} }
st = _Py_SymtableStringObjectFlags(str, filename, start, &cf); st = _Py_SymtableStringObjectFlags(str, filename, start, &cf);
Py_DECREF(filename);
Py_XDECREF(source_copy); Py_XDECREF(source_copy);
if (st == NULL) { if (st == NULL) {
return NULL; return NULL;

View file

@ -745,7 +745,7 @@ builtin_chr(PyObject *module, PyObject *i)
compile as builtin_compile compile as builtin_compile
source: object source: object
filename: object(converter="PyUnicode_FSDecoder") filename: unicode_fs_decoded
mode: str mode: str
flags: int = 0 flags: int = 0
dont_inherit: bool = False dont_inherit: bool = False
@ -771,7 +771,7 @@ static PyObject *
builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename,
const char *mode, int flags, int dont_inherit, const char *mode, int flags, int dont_inherit,
int optimize, int feature_version) int optimize, int feature_version)
/*[clinic end generated code: output=b0c09c84f116d3d7 input=cc78e20e7c7682ba]*/ /*[clinic end generated code: output=b0c09c84f116d3d7 input=8f0069edbdac381b]*/
{ {
PyObject *source_copy; PyObject *source_copy;
const char *str; const char *str;
@ -889,7 +889,6 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename,
error: error:
result = NULL; result = NULL;
finally: finally:
Py_DECREF(filename);
return result; return result;
} }

View file

@ -296,7 +296,7 @@ builtin_compile(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObj
PyObject *argsbuf[7]; PyObject *argsbuf[7];
Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 3; Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 3;
PyObject *source; PyObject *source;
PyObject *filename; PyObject *filename = NULL;
const char *mode; const char *mode;
int flags = 0; int flags = 0;
int dont_inherit = 0; int dont_inherit = 0;
@ -367,6 +367,9 @@ skip_optional_kwonly:
return_value = builtin_compile_impl(module, source, filename, mode, flags, dont_inherit, optimize, feature_version); return_value = builtin_compile_impl(module, source, filename, mode, flags, dont_inherit, optimize, feature_version);
exit: exit:
/* Cleanup for filename */
Py_XDECREF(filename);
return return_value; return return_value;
} }
@ -1274,4 +1277,4 @@ builtin_issubclass(PyObject *module, PyObject *const *args, Py_ssize_t nargs)
exit: exit:
return return_value; return return_value;
} }
/*[clinic end generated code: output=c0b72519622c849e input=a9049054013a1b77]*/ /*[clinic end generated code: output=7eada753dc2e046f input=a9049054013a1b77]*/

View file

@ -916,6 +916,26 @@ def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> st
return super().parse_arg(argname, displayname, limited_capi=limited_capi) return super().parse_arg(argname, displayname, limited_capi=limited_capi)
class _unicode_fs_converter_base(CConverter):
type = 'PyObject *'
def converter_init(self) -> None:
if self.default is not unspecified:
fail(f"{self.__class__.__name__} does not support default values")
self.c_default = 'NULL'
def cleanup(self) -> str:
return f"Py_XDECREF({self.parser_name});"
class unicode_fs_encoded_converter(_unicode_fs_converter_base):
converter = 'PyUnicode_FSConverter'
class unicode_fs_decoded_converter(_unicode_fs_converter_base):
converter = 'PyUnicode_FSDecoder'
@add_legacy_c_converter('u') @add_legacy_c_converter('u')
@add_legacy_c_converter('u#', zeroes=True) @add_legacy_c_converter('u#', zeroes=True)
@add_legacy_c_converter('Z', accept={str, NoneType}) @add_legacy_c_converter('Z', accept={str, NoneType})