diff --git a/Include/internal/pycore_bytesobject.h b/Include/internal/pycore_bytesobject.h index 443bdb26ff8738c..8f764f0fa6d6e12 100644 --- a/Include/internal/pycore_bytesobject.h +++ b/Include/internal/pycore_bytesobject.h @@ -81,6 +81,13 @@ extern int _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize); extern int _PyBytes_IsMutable(PyObject *obj); #endif +#ifdef Py_DEBUG +extern void _PyBytes_CheckOverflow( + PyObject *op, + void *addr, + const char *type_name); +#endif + /* --- PyBytesWriter ------------------------------------------------------ */ struct PyBytesWriter { diff --git a/Lib/test/test_capi/test_bytearray.py b/Lib/test/test_capi/test_bytearray.py index cb7ad8b22252d9f..638a29f026cd97a 100644 --- a/Lib/test/test_capi/test_bytearray.py +++ b/Lib/test/test_capi/test_bytearray.py @@ -1,6 +1,9 @@ import sys +import textwrap import unittest +from test import support from test.support import import_helper +from test.support.script_helper import assert_python_failure _testlimitedcapi = import_helper.import_module('_testlimitedcapi') from _testcapi import PY_SSIZE_T_MIN, PY_SSIZE_T_MAX @@ -172,6 +175,26 @@ def test_resize(self): # CRASHES resize(object(), 0) # CRASHES resize(NULL, 0) + @unittest.skipUnless(support.Py_DEBUG, 'need debug build (Py_DEBUG)') + def test_detect_overflow(self): + # Test detection of buffer overflow + size = 123 # bytes + overflow = 1 # bytes + code = textwrap.dedent(f''' + from test.support import SuppressCrashReport + import _testcapi + + size = {size} + overflow = {overflow} + with SuppressCrashReport(): + # Trigger a buffer overflow in a new bytearray + ba = _testcapi.bytearray_overflow(size, overflow) + ba = None + ''') + proc = assert_python_failure('-c', code) + self.assertIn(b'Buffer overflow detected in bytearray object', proc.err) + self.assertIn(f'at position {size}'.encode(), proc.err) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_capi/test_bytes.py b/Lib/test/test_capi/test_bytes.py index a0006ea35e21fe7..12a1e88eac82d9a 100644 --- a/Lib/test/test_capi/test_bytes.py +++ b/Lib/test/test_capi/test_bytes.py @@ -317,6 +317,26 @@ def test_join(self): with self.assertRaises(SystemError): bytes_join(b'', NULL) + @unittest.skipUnless(support.Py_DEBUG, 'need debug build (Py_DEBUG)') + def test_detect_overflow(self): + # Test detection of buffer overflow + size = 123 # bytes + overflow = 1 # bytes + code = textwrap.dedent(f''' + from test.support import SuppressCrashReport + import _testcapi + + size = {size} + overflow = {overflow} + with SuppressCrashReport(): + # Trigger a buffer overflow in a new bytes + ba = _testcapi.bytes_overflow(size, overflow) + ba = None + ''') + proc = assert_python_failure('-c', code) + self.assertIn(b'Buffer overflow detected in bytes object', proc.err) + self.assertIn(f'at position {size}'.encode(), proc.err) + def get_data_canary(writer): size = writer.get_size() + 1 diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-09-14-23-16-26.gh-issue-156939.oZqlSt.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-14-23-16-26.gh-issue-156939.oZqlSt.rst new file mode 100644 index 000000000000000..212679fc0a79bfb --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-14-23-16-26.gh-issue-156939.oZqlSt.rst @@ -0,0 +1,3 @@ +When Python is built in debug mode, :class:`bytes` and :class:`bytearray` +destructors now check if the trailing null byte has been overridden to detect +buffer overflow. Patch by Victor Stinner. diff --git a/Modules/_testcapi/bytes.c b/Modules/_testcapi/bytes.c index 79effcad40090e0..e3d966b3bb18968 100644 --- a/Modules/_testcapi/bytes.c +++ b/Modules/_testcapi/bytes.c @@ -528,6 +528,45 @@ test_byteswriter_ptr(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } +static PyObject * +bytes_overflow(PyObject *Py_UNUSED(module), PyObject *args) +{ + Py_ssize_t alloc, overflow = 1; + if (!PyArg_ParseTuple(args, "n|n", &alloc, &overflow)) + return NULL; + + PyObject *bytes = PyObject_CallFunction((PyObject*)&PyBytes_Type, "n", alloc); + if (bytes == NULL) { + return NULL; + } + + char *data = PyBytes_AS_STRING(bytes); + Py_ssize_t size = PyBytes_GET_SIZE(bytes); + memset(data, 'x', size); + memset(data + size, '#', overflow); // Buffer overflow! + return bytes; +} + + +static PyObject * +bytearray_overflow(PyObject *Py_UNUSED(module), PyObject *args) +{ + Py_ssize_t alloc, overflow = 1; + if (!PyArg_ParseTuple(args, "n|n", &alloc, &overflow)) + return NULL; + + PyObject *bytearray = PyObject_CallFunction((PyObject*)&PyByteArray_Type, "n", alloc); + if (bytearray == NULL) { + return NULL; + } + + char *data = PyByteArray_AS_STRING(bytearray); + Py_ssize_t size = PyByteArray_GET_SIZE(bytearray); + memset(data + size, '#', overflow); // Buffer overflow! + return bytearray; +} + + static PyMethodDef test_methods[] = { {"bytes_resize", bytes_resize, METH_VARARGS}, {"bytes_join", bytes_join, METH_VARARGS}, @@ -535,6 +574,8 @@ static PyMethodDef test_methods[] = { {"byteswriter_resize", byteswriter_resize, METH_NOARGS}, {"byteswriter_highlevel", byteswriter_highlevel, METH_NOARGS}, {"test_byteswriter_ptr", test_byteswriter_ptr, METH_NOARGS}, + {"bytes_overflow", bytes_overflow, METH_VARARGS}, + {"bytearray_overflow", bytearray_overflow, METH_VARARGS}, {NULL}, }; diff --git a/Modules/_testcapi/mem.c b/Modules/_testcapi/mem.c index 4ae6a60ff39d157..ba1462481231b50 100644 --- a/Modules/_testcapi/mem.c +++ b/Modules/_testcapi/mem.c @@ -448,6 +448,7 @@ test_pyobject_new(PyObject *self, PyObject *Py_UNUSED(ignored)) if (obj == NULL) { goto alloc_failed; } + memset(PyBytes_AS_STRING(obj), 0, 3 + 1); // +1 for the null byte Py_DECREF(obj); // PyObject_NEW_VAR() @@ -455,6 +456,7 @@ test_pyobject_new(PyObject *self, PyObject *Py_UNUSED(ignored)) if (obj == NULL) { goto alloc_failed; } + memset(PyBytes_AS_STRING(obj), 0, 3 + 1); // +1 for the null byte Py_DECREF(obj); Py_RETURN_NONE; diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index de30c6118ba176b..16c384035478185 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -1272,6 +1272,12 @@ static void bytearray_dealloc(PyObject *op) { PyByteArrayObject *self = _PyByteArray_CAST(op); +#ifdef Py_DEBUG + if (self->ob_bytes_object != NULL) { + _PyBytes_CheckOverflow(self->ob_bytes_object, op, "bytearray"); + } +#endif + if (self->ob_exports > 0) { PyErr_SetString(PyExc_SystemError, "deallocated bytearray object has exported buffers"); diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 117d8b56017b64a..4f33b14a197eab0 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -3191,12 +3191,42 @@ bytes_iteritem(PyObject *obj, Py_ssize_t index) return (_PyObjectIndexPair) { .object = l, .index = index + 1 }; } +#ifdef Py_DEBUG +void +_PyBytes_CheckOverflow(PyObject *self, void *addr, const char *type_name) +{ + // Make sure that the trailing null byte was not modified + char *data = PyBytes_AS_STRING(self); + Py_ssize_t size = PyBytes_GET_SIZE(self); + if (data[size] != '\0') { + _Py_FatalErrorFormat(__func__, + "Buffer overflow detected in %s object %p " + "at position %zd", + type_name, addr, size); + } +} + + +static void +bytes_dealloc(PyObject *op) +{ + PyBytesObject *self = _PyBytes_CAST(op); + _PyBytes_CheckOverflow(op, op, "bytes"); + Py_TYPE(self)->tp_free((PyObject *)self); +} +#endif + + PyTypeObject PyBytes_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "bytes", PyBytesObject_SIZE, sizeof(char), +#ifdef Py_DEBUG + bytes_dealloc, /* tp_dealloc */ +#else 0, /* tp_dealloc */ +#endif 0, /* tp_vectorcall_offset */ 0, /* tp_getattr */ 0, /* tp_setattr */ @@ -3665,6 +3695,18 @@ byteswriter_write_canary_byte(PyBytesWriter *writer) unsigned char *data = (unsigned char*)byteswriter_data(writer); data[writer->size] = PyBytesWriter_CANARY_BYTE; } + + +static void +byteswriter_reset_trailing_byte(PyBytesWriter *writer) +{ + // PyBytesWriter writes non-zero canary byte as the last byte. + // bytes/bytearray expects the last byte to be a null byte. + // Reset the last byte to null for bytes/bytearray. + Py_ssize_t allocated = byteswriter_allocated(writer); + char *data = byteswriter_data(writer); + data[allocated] = '\0'; +} #endif @@ -3814,6 +3856,9 @@ PyBytesWriter_Discard(PyBytesWriter *writer) #ifdef Py_DEBUG byteswriter_check_canary_byte(writer); + if (writer->obj != NULL) { + byteswriter_reset_trailing_byte(writer); + } #endif Py_XDECREF(writer->obj); @@ -3838,16 +3883,7 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) } #ifdef Py_DEBUG - // Check for buffer overflow 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; @@ -3855,6 +3891,11 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) result = bytes_get_empty(); } else if (writer->obj != NULL) { + // Truncate the bytes/bytearray object if needed +#ifdef Py_DEBUG + byteswriter_reset_trailing_byte(writer); +#endif + if (writer->use_bytearray) { if (size != PyByteArray_GET_SIZE(writer->obj)) { if (PyByteArray_Resize(writer->obj, size)) { @@ -3868,25 +3909,28 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size) goto error; } } + + if (size == 1) { + // Get the single byte singleton + unsigned char ch = PyBytes_AS_STRING(writer->obj)[0]; + PyObject *op = (PyObject*)CHARACTER(ch); + assert(_Py_IsImmortal(op)); + Py_SETREF(writer->obj, op); + } } result = writer->obj; writer->obj = NULL; - - if (size == 1 && !writer->use_bytearray) { - // Get the single byte singleton - unsigned char ch = PyBytes_AS_STRING(result)[0]; - PyObject *op = (PyObject*)CHARACTER(ch); - assert(_Py_IsImmortal(op)); - Py_SETREF(result, op); - } - } - else if (writer->use_bytearray) { - result = PyByteArray_FromStringAndSize(writer->small_buffer, size); } else { - // The function returns single byte singleton if size equals 1 - result = PyBytes_FromStringAndSize(writer->small_buffer, size); + // Create an object from the small buffer + if (writer->use_bytearray) { + result = PyByteArray_FromStringAndSize(writer->small_buffer, size); + } + else { + // The function returns single byte singleton if size equals 1 + result = PyBytes_FromStringAndSize(writer->small_buffer, size); + } } #ifdef Py_DEBUG