gh-157710: Detect overflow in PyUnicodeWriter_Finish() (#157715)

Check if the trailing null character has been modified to detect
buffer overflow in C extensions.

_PyUnicode_CheckConsistency() now always check if the trailing null
character has been overridden to detect buffer overflow. Previously,
it was only been checked if check_content parameter was non-zero.

Author: vstinner

Lib/test/test_capi/test_unicode.py

@@ -1,7 +1,9 @@
-import unittest
 import sys
+import textwrap
+import unittest
 from test import support
 from test.support import threading_helper
+from test.support.script_helper import assert_python_failure
 
 try:
     import _testcapi
@@ -1992,6 +1994,22 @@ def test_singletons(self):
                 writer.write_substring(ch + 'xxx', 0, 1)
                 self.assertIs(writer.finish(), ch)
 
+    @unittest.skipUnless(support.Py_DEBUG, 'need debug build (Py_DEBUG)')
+    def test_detect_overflow(self):
+        # Test detection of buffer overflow
+        code = textwrap.dedent('''
+            from test.support import SuppressCrashReport
+            import _testinternalcapi
+
+            SuppressCrashReport().__enter__()
+            _testinternalcapi.unicodewriter_overflow()
+        ''')
+        proc = assert_python_failure('-c', code)
+        self.assertIn(b'Buffer overflow detected in PyUnicodeWriter', proc.err)
+        # Do not test the position value since it depends on the overallocation
+        # strategy which depends on the operating system
+        self.assertIn(f'at position '.encode(), proc.err)
+
 
 @unittest.skipIf(ctypes is None, 'need ctypes')
 class PyUnicodeWriterFormatTest(unittest.TestCase):

Misc/NEWS.d/next/C_API/2026-09-18-00-29-49.gh-issue-157710.Q_AD8D.rst

@@ -0,0 +1,3 @@
+When Python is built in debug mode, :c:func:`PyUnicodeWriter_Finish` now
+checks if the trailing null byte has been overridden to detect buffer
+overflow. Patch by Victor Stinner.

Modules/_testinternalcapi.c

@@ -3206,6 +3206,28 @@ test_thread_state_ensure_from_view_interp_switch(PyObject *self, PyObject *unuse
     Py_RETURN_NONE;
 }
 
+static PyObject *
+unicodewriter_overflow(PyObject *self, PyObject *unused)
+{
+    PyUnicodeWriter *writer = PyUnicodeWriter_Create(0);
+    if (writer == NULL) {
+        return NULL;
+    }
+    if (PyUnicodeWriter_WriteASCII(writer, "hello", -1) < 0) {
+        PyUnicodeWriter_Discard(writer);
+        return NULL;
+    }
+
+    _PyUnicodeWriter *impl = (_PyUnicodeWriter*)writer;
+    PyObject *buffer = impl->buffer;
+    Py_ssize_t index = PyUnicode_GET_LENGTH(buffer);
+    PyUnicode_WRITE(impl->kind, impl->data, index, '#');  // overflow!
+
+    // Spoiler: the function doesn't return if an overflow is detected
+    // in debug mode
+    return PyUnicodeWriter_Finish(writer);
+}
+
 /* Self interrupting context manager */
 
 typedef struct {
@@ -3393,6 +3415,7 @@ static PyMethodDef module_functions[] = {
     {"test_interp_guard_countdown", test_interp_guard_countdown, METH_NOARGS},
     {"test_interp_view_countdown", test_interp_view_countdown, METH_NOARGS},
     {"test_thread_state_ensure_from_view_interp_switch", test_thread_state_ensure_from_view_interp_switch, METH_NOARGS},
+    {"unicodewriter_overflow", unicodewriter_overflow, METH_NOARGS},
     {NULL, NULL} /* sentinel */
 };
 

Objects/unicode_writer.c

@@ -608,6 +608,20 @@ _PyUnicodeWriter_Finish(_PyUnicodeWriter *writer)
 {
     PyObject *str;
 
+#ifdef Py_DEBUG
+    // Check for buffer overflow
+    if (writer->buffer != NULL) {
+        Py_ssize_t pos = PyUnicode_GET_LENGTH(writer->buffer);
+        Py_UCS4 ch = PyUnicode_READ_CHAR(writer->buffer, pos);
+        if (ch != 0) {
+            _Py_FatalErrorFormat(__func__,
+                                 "Buffer overflow detected in "
+                                 "PyUnicodeWriter %p at position %zd",
+                                 writer, pos);
+        }
+    }
+#endif
+
     if (writer->pos == 0) {
         Py_CLEAR(writer->buffer);
         return _PyUnicode_GetEmpty();
@@ -618,6 +632,7 @@ _PyUnicodeWriter_Finish(_PyUnicodeWriter *writer)
 
     if (writer->readonly) {
         assert(PyUnicode_GET_LENGTH(str) == writer->pos);
+        assert(_PyUnicode_CheckConsistency(str, 1));
         return str;
     }
 

Objects/unicodeobject.c

@@ -601,7 +601,6 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content)
 # define CHECK_IF_FT(expr) (void)(expr)
 #endif
 
-
     assert(op != NULL);
     CHECK(PyUnicode_Check(op));
 
@@ -647,13 +646,12 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content)
     }
 
     /* check that the best kind is used: O(n) operation */
+    const void *data = PyUnicode_DATA(ascii);
     if (check_content) {
         Py_ssize_t i;
         Py_UCS4 maxchar = 0;
-        const void *data;
         Py_UCS4 ch;
 
-        data = PyUnicode_DATA(ascii);
         for (i=0; i < ascii->length; i++)
         {
             ch = PyUnicode_READ(kind, data, i);
@@ -676,9 +674,12 @@ _PyUnicode_CheckConsistency(PyObject *op, int check_content)
             CHECK(maxchar >= 0x10000);
             CHECK(maxchar <= MAX_UNICODE);
         }
-        CHECK(PyUnicode_READ(kind, data, ascii->length) == 0);
     }
 
+    // Detect buffer overflow: check if the trailing null character
+    // has been overridden
+    CHECK(PyUnicode_READ(kind, data, ascii->length) == 0);
+
     /* Check interning state */
 #ifdef Py_DEBUG
     // Note that we do not check `_Py_IsImmortal(op)` in the GIL-enabled build