diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index 38cda931e7d54f3..3cc2038ddab54ab 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -1,5 +1,7 @@ +import textwrap import unittest from test.support import import_helper +from test.support.script_helper import assert_python_failure _testlimitedcapi = import_helper.import_module('_testlimitedcapi') _testcapi = import_helper.import_module('_testcapi') @@ -378,15 +380,6 @@ def test_format_i(self): writer.format_i(b'y=%i', 456) self.assertEqual(writer.finish(), self.result_type(b'x=123, y=456')) - def test_example_abc(self): - self.assertEqual(_testcapi.byteswriter_abc(), b'abc') - - def test_example_resize(self): - self.assertEqual(_testcapi.byteswriter_resize(), b'Hello World') - - def test_example_highlevel(self): - self.assertEqual(_testcapi.byteswriter_highlevel(), b'Hello World!') - class BytesWriterTest(BaseWriterTest, unittest.TestCase): result_type = bytes @@ -424,6 +417,34 @@ def test_singletons(self): writer.write_bytes(unused_text, len(unused_text)) self.assertIs(writer.finish_with_size(1), singletons[ch]) + def test_example_abc(self): + self.assertEqual(_testcapi.byteswriter_abc(), b'abc') + + def test_example_resize(self): + self.assertEqual(_testcapi.byteswriter_resize(), b'Hello World') + + def test_example_highlevel(self): + self.assertEqual(_testcapi.byteswriter_highlevel(), b'Hello World!') + + def test_canary_byte(self): + small_buffer = _testcapi.PyBytesWriter_small_buffer + large_size = small_buffer * 10 + + # Test small buffer and large buffer + for size in (0, 3, large_size): + with self.subTest(size=size): + code = textwrap.dedent(f""" + from test.support import SuppressCrashReport + import _testcapi + size = {size} + data = b'x' * size + with SuppressCrashReport(): + _testcapi.byteswriter_test_canary_byte(data) + """) + proc = assert_python_failure('-c', code) + self.assertIn(b'Buffer overflow detected in PyBytesWriter', + proc.err) + class ByteArrayWriterTest(BaseWriterTest, unittest.TestCase): result_type = bytearray diff --git a/Misc/NEWS.d/next/C_API/2026-09-04-16-41-07.gh-issue-156939.bKaQuE.rst b/Misc/NEWS.d/next/C_API/2026-09-04-16-41-07.gh-issue-156939.bKaQuE.rst new file mode 100644 index 000000000000000..57c9a0ab46e9046 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-09-04-16-41-07.gh-issue-156939.bKaQuE.rst @@ -0,0 +1,2 @@ +When Python is built in debug mode, :c:type:`PyBytesWriter` now detects +buffer overflow. Patch by Victor Stinner. diff --git a/Modules/_testcapi/bytes.c b/Modules/_testcapi/bytes.c index a868c684cc987cd..1a3b49b28c10c7e 100644 --- a/Modules/_testcapi/bytes.c +++ b/Modules/_testcapi/bytes.c @@ -151,7 +151,7 @@ writer_write_bytes(PyObject *self_raw, PyObject *args) return NULL; } - char *bytes; + const char *bytes; Py_ssize_t unused_size, size; if (!PyArg_ParseTuple(args, "y#n", &bytes, &unused_size, &size)) { return NULL; @@ -353,12 +353,45 @@ byteswriter_highlevel(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } +// Trigger a buffer overflow on purpose to test the canary byte feature +// which detects buffer overflow +static PyObject * +byteswriter_test_canary_byte(PyObject *Py_UNUSED(module), PyObject *args) +{ + const char *str; + Py_ssize_t len; + if (!PyArg_ParseTuple(args, "s#", &str, &len)) { + return NULL; + } + + PyBytesWriter *writer = PyBytesWriter_Create(0); + if (writer == NULL) { + goto error; + } + if (PyBytesWriter_Grow(writer, len) < 0) { + goto error; + } + char *data = PyBytesWriter_GetData(writer); + if (len) { + memcpy(data, str, len); + } + data[len] = '#'; // Overflow! + + return PyBytesWriter_Finish(writer); + +error: + PyBytesWriter_Discard(writer); + return NULL; +} + + static PyMethodDef test_methods[] = { {"bytes_resize", bytes_resize, METH_VARARGS}, {"bytes_join", bytes_join, METH_VARARGS}, {"byteswriter_abc", byteswriter_abc, METH_NOARGS}, {"byteswriter_resize", byteswriter_resize, METH_NOARGS}, {"byteswriter_highlevel", byteswriter_highlevel, METH_NOARGS}, + {"byteswriter_test_canary_byte", byteswriter_test_canary_byte, METH_VARARGS}, {NULL}, }; diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 4c3da93f1019709..77c7340db778573 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -2995,6 +2995,8 @@ _PyBytes_FromList(PyObject *x) } char *str = PyBytesWriter_GetData(writer); size = _PyBytesWriter_GetAllocated(writer); + str = _PyBytesWriter_ResizeAndUpdatePointer(writer, size, str); + assert(str != NULL); for (Py_ssize_t i = 0; i < PyList_GET_SIZE(x); i++) { PyObject *item = _PyList_GetItemRef((PyListObject *)x, i); @@ -3018,6 +3020,8 @@ _PyBytes_FromList(PyObject *x) goto error; } size = _PyBytesWriter_GetAllocated(writer); + str = _PyBytesWriter_ResizeAndUpdatePointer(writer, size, str); + assert(str != NULL); } *str++ = (char) value; } @@ -3077,6 +3081,8 @@ _PyBytes_FromIterator(PyObject *it, PyObject *x) } char *str = PyBytesWriter_GetData(writer); size = _PyBytesWriter_GetAllocated(writer); + str = _PyBytesWriter_ResizeAndUpdatePointer(writer, size, str); + assert(str != NULL); /* Run the iterator to exhaustion */ for (i = 0; ; i++) { @@ -3111,6 +3117,8 @@ _PyBytes_FromIterator(PyObject *it, PyObject *x) goto error; } size = _PyBytesWriter_GetAllocated(writer); + str = _PyBytesWriter_ResizeAndUpdatePointer(writer, size, str); + assert(str != NULL); } *str++ = (char) value; } @@ -3589,6 +3597,10 @@ _PyBytes_RepeatBuffer(char* dest, Py_ssize_t len_dest, // --- PyBytesWriter API ----------------------------------------------------- +// Use a value different than NUL (0) to be able to detect overflow writing +// one extra NUL byte which is a common error. +#define PyBytesWriter_CANARY_BYTE PYMEM_DEADBYTE + static inline char* byteswriter_data(PyBytesWriter *writer) { @@ -3611,6 +3623,49 @@ byteswriter_allocated(PyBytesWriter *writer) } +#ifdef Py_DEBUG +static int +byteswriter_has_canary_byte(PyBytesWriter *writer) +{ + if (writer->obj == NULL) { + return (writer->size < byteswriter_allocated(writer)); + } + else { + // For bytes or bytearray, use the trailing NUL byte + // as the canary byte. + return 1; + } +} + + +static void +byteswriter_check_canary_byte(PyBytesWriter *writer) +{ + assert(byteswriter_has_canary_byte(writer)); + + const unsigned char *data = (const unsigned char*)byteswriter_data(writer); + unsigned char canary = data[writer->size]; + if (canary != PyBytesWriter_CANARY_BYTE) { + _Py_FatalErrorFormat(__func__, + "Buffer overflow detected in PyBytesWriter %p: " + "one byte written after the buffer " + "(at position %zd)", + writer, writer->size); + } +} + + +static void +byteswriter_write_canary_byte(PyBytesWriter *writer) +{ + assert(byteswriter_has_canary_byte(writer)); + + unsigned char *data = (unsigned char*)byteswriter_data(writer); + data[writer->size] = PyBytesWriter_CANARY_BYTE; +} +#endif + + #ifdef MS_WINDOWS /* On Windows, overallocate by 50% is the best factor */ # define OVERALLOCATE_FACTOR 2 @@ -3625,6 +3680,17 @@ byteswriter_resize(PyBytesWriter *writer, Py_ssize_t size, int resize) assert(size >= 0); Py_ssize_t old_allocated = byteswriter_allocated(writer); +#ifdef Py_DEBUG + if (writer->obj == NULL) { + // If the small_buffer is used, reserve one byte for the canary byte. + if (size <= (old_allocated - 1)) { + return 0; + } + } + else + // bytes and bytearray always allocates one extra byte for a trailing + // NUL byte: use this byte as the canary byte. +#endif if (size <= old_allocated) { return 0; } @@ -3713,9 +3779,12 @@ byteswriter_create(Py_ssize_t size, int use_bytearray) } writer->size = size; } + #ifdef Py_DEBUG memset(byteswriter_data(writer), 0xff, byteswriter_allocated(writer)); + byteswriter_write_canary_byte(writer); #endif + return writer; } @@ -3747,6 +3816,17 @@ PyBytesWriter_Discard(PyBytesWriter *writer) PyObject* PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) { +#ifdef Py_DEBUG + byteswriter_check_canary_byte(writer); + if (writer->obj != NULL) { + // byteswriter_write_canary_byte() can override the trailing NUL byte. + // So reset the trailing NUL byte to NUL. + Py_ssize_t allocated = byteswriter_allocated(writer); + char *data = byteswriter_data(writer); + data[allocated] = '\0'; + } +#endif + PyObject *result; if (size == 0) { result = bytes_get_empty(); @@ -3846,6 +3926,9 @@ PyBytesWriter_Resize(PyBytesWriter *writer, Py_ssize_t size) return -1; } writer->size = size; +#ifdef Py_DEBUG + byteswriter_write_canary_byte(writer); +#endif return 0; } @@ -3879,6 +3962,9 @@ PyBytesWriter_Grow(PyBytesWriter *writer, Py_ssize_t size) return -1; } writer->size = size; +#ifdef Py_DEBUG + byteswriter_write_canary_byte(writer); +#endif return 0; }