Undefined Behavior in _PyUnicodeWriter_WriteASCIIString: NULL pointer passed to memcpy when len is 0

[ashm-dev]

Bug report

Bug description:

The function _PyUnicodeWriter_WriteASCIIString in Objects/unicodeobject.c (or Objects/unicode_writer.c in some versions) contains a potential Undefined Behavior. When len is 0 and ascii is NULL, it calls memcpy with a NULL source pointer.

According to the C standard, passing NULL to memcpy is undefined even if the count is zero.

Proof of Concept

Clang's UndefinedBehaviorSanitizer (UBSan) reports:
../Objects/unicode_writer.c:494:36: runtime error: null pointer passed as argument 2, which is declared to never be null

This happens in the following block:

    case PyUnicode_1BYTE_KIND:
    {
        const Py_UCS1 *str = (const Py_UCS1 *)ascii;
        Py_UCS1 *data = writer->data;
        memcpy(data + writer->pos, str, len); // <--- UB if str is NULL and len is 0
        break;
    }

Mitigation

The function should return early if len == 0. This is a common pattern in CPython to avoid unnecessary work and prevent UB with memory functions.

    if (len == -1)
        len = strlen(ascii);
    if (len == 0)
        return 0;

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-146196: Fix potential Undefined Behavior in _PyUnicodeWriter_WriteASCIIString #146201
• [3.14] gh-146196: Fix Undefined Behavior in _PyUnicodeWriter_WriteASCIIString() (#146201) #146220

[sergey-miryanov]

Could you check what versions affected and add MRE?

[ashm-dev]

This is only in the main branch, and I'll push a PR with the fix myself shortly

[ronaldoussoren]

Note that this will become valid code, see https://developers.redhat.com/articles/2024/12/11/making-memcpynull-null-0-well-defined#null_pointer_arithmetic. Both gcc and clang claim to conform this accepted proposal.

[ashm-dev]

@ronaldoussoren Am I the only one, C2y?
I looked into that proposal (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf) too, BUT it’s for newer versions of C and will only be available in newer compilers. This problem is already relevant right now.

[vstinner]

Could you check what versions affected and add MRE?

@serhiy-storchaka recently added writer.write_ascii(NULL, 0) test to Lib/test/test_capi/test_unicode.py. It should trigger the undefined behavior.

[vstinner]

Ah yes, I confirm that the newly added test triggers the undefined behavior:

$ ./configure --with-pydebug --with-undefined-behavior-sanitizer
$ make
$ ./python -m test -v test_capi.test_unicode -m test_ascii
(...)
test_ascii (test.test_capi.test_unicode.PyUnicodeWriterTest.test_ascii) ...
Objects/unicode_writer.c:494:9: runtime error: null pointer passed as argument 2, which is declared to never be null
(...)

So there is no need to add another test.

The good news is that other PyUnicodeWriter functions don't emit undefined behavior (according to existing tests).

[serhiy-storchaka]

The test was backported to 3.14.

Note that we get a crash for non-zero size. The case for zero size was not explicitly supported, the test was only added to prevent future regressions.

We should ether make the behavior well defined by adding the explicit check for 0 size, or comment out the test. We can also explicitly raise SystemError for NULL. Either is fine. But since there is no use case for passing NULL, commenting out the test may be preferable -- it adds zero overhead.