Repository navigation
Double-free in Argument Clinic str_converter generated code #99240
Description
Argument Clinic str_converter generate such code when encoding is set
(see function test_str_converter_encoding in file Lib/test/clinic.test):
/* -- snip -- */
if (!_PyArg_ParseStack(args, nargs, "esesetes#et#:test_str_converter_encoding",
"idna", &a, "idna", &b, "idna", &c, "idna", &d, &d_length, "idna", &e, &e_length)) {
goto exit;
}
return_value = test_str_converter_encoding_impl(module, a, b, c, d, d_length, e, e_length);
exit:
/* Cleanup for a */
if (a) {
PyMem_FREE(a);
}
/* Cleanup for b */
if (b) {
PyMem_FREE(b);
}
/* Cleanup for c */
if (c) {
PyMem_FREE(c);
}
/* -- snip -- */If parsing a successes, a will be assigned an address points to an allocated memory.
After that, if parsing b fails, the memory which a points to is freed by function _PyArg_ParseStack,
and _PyArg_ParseStack returns 0, then control flow goes to label "exit".
At this time, a is not NULL, so the memory it points to is freed again, which cause a double-free problem and a runtime crash.
This bug is found in #96178 "Argument Clinic functional test".
Activity
[-]Double-fre in Argument clinic `str_converter` generated code[/-][+]Double-free in Argument clinic `str_converter` generated code[/+][-]Double-free in Argument clinic `str_converter` generated code[/-][+]Double-free in Argument Clinic `str_converter` generated code[/+]Add PoC of pythonGH-99240
colorfulappl commentedon Nov 8, 2022
There are two ways to fix this bug,
_PyArg_ParseStack,as gh-99240: Fix double-free bug in Argument Clinic str_converter generated code #99241 have done.
_PyArg_ParseStackparses failed, assign all the parsed arguments to "NULL" after they are freed, this should be done in_PyArg_ParseStack.gh-99240: Fix double-free bug in Argument Clinic str_converter genera…
hauntsaninja commentedon Nov 29, 2022
Thanks for reporting and fixing, looks like this has been completed
erlend-aasland commentedon Nov 29, 2022
We should consider backporting this, IMO.
colorfulappl commentedon Nov 30, 2022
Should we backport #96002 before backporting this?
And I made a new bug fix #99890 .
erlend-aasland commentedon Nov 30, 2022
Yes, I think we should. Although that PR introduced a new C file, it expands the Argument Clinic coverage considerably, so I think it is definitely worth it.
Great :)
gh-99240: Reset pointer to NULL when the pointed memory is freed in a…
1 remaining item
Merge remote-tracking branch 'origin/main' into bpo_34697
Merge branch 'main' into fastmock
[3.11] pythongh-99240: Fix double-free bug in Argument Clinic str_con…
[3.10] pythongh-99240: Fix double-free bug in Argument Clinic str_con…
[3.11] gh-99240: Fix double-free bug in Argument Clinic str_converter…
[3.10] gh-99240: Fix double-free bug in Argument Clinic str_converter…
[3.11] pythongh-99240: Reset pointer to NULL when the pointed memory …
[3.10] pythongh-99240: Reset pointer to NULL when the pointed memory …
[3.11] gh-99240: Reset pointer to NULL when the pointed memory is fre…
[3.10] gh-99240: Reset pointer to NULL when the pointed memory is fre…
kumaraditya303 commentedon Dec 21, 2022
Thanks for working on this, all PRs have been merged.
erlend-aasland commentedon Dec 23, 2022
Yes, thanks for all your good work on argument clinic, @colorfulappl! And thank you Kumar for landing these PRs; I've had a hard time keeping up with CPython dev lately.
pythongh-99240: Reset pointer to NULL when the pointed memory is free…
gh-99240: Fix double-free bug in Argument Clinic str_converter genera…