Double-free in Argument Clinic `str_converter` generated code

[colorfulappl]

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 https://github.com/python/cpython/pull/96178 "Argument Clinic functional test".

• PR: gh-99241

[colorfulappl]

There are two ways to fix this bug,

1. Avoid free any parsed arguments if an error occurred in function _PyArg_ParseStack, as https://github.com/python/cpython/pull/99241 have done.
2. If function _PyArg_ParseStack parses failed, assign all the parsed arguments to "NULL" after they are freed, this should be done in _PyArg_ParseStack.

[hauntsaninja]

Thanks for reporting and fixing, looks like this has been completed

[erlend-aasland]

We should consider backporting this, IMO.

[colorfulappl]

We should consider backporting this, IMO.

Should we backport #96002 before backporting this?

And I made a new bug fix #99890 .

[erlend-aasland]

Should we backport #96002 before backporting this?

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.

And I made a new bug fix #99890 .

Great :)

[kumaraditya303]

Thanks for working on this, all PRs have been merged.

[erlend-aasland]

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.