FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-156939: Detect buffer overflow in bytes and bytearray (#157529) · python/cpython@2d1007f · GitHub

  1. python
  2. cpython

Repository navigation

Commit 2d1007f

Browse files
vstinner
authored
gh-156939: Detect buffer overflow in bytes and bytearray (#157529)
When Python is built in debug mode, bytes an bytearray destructors now check if the trailing null byte has been overridden to detect overflow. Add bytes_dealloc() to implement the check. Add _PyBytes_CheckOverflow() to share code
1 parent d95f295 commit 2d1007f

8 files changed

Lines changed: 168 additions & 22 deletions

File tree

‎Include/internal/pycore_bytesobject.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ extern int _PyBytes_ResizeKeepOnError(PyObject **pv, Py_ssize_t newsize);
8181
extern int _PyBytes_IsMutable(PyObject *obj);
8282
#endif
8383

84+
#ifdef Py_DEBUG
85+
extern void _PyBytes_CheckOverflow(
86+
PyObject *op,
87+
void *addr,
88+
const char *type_name);
89+
#endif
90+
8491
/* --- PyBytesWriter ------------------------------------------------------ */
8592

8693
struct PyBytesWriter {

‎Lib/test/test_capi/test_bytearray.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import sys
2+
import textwrap
23
import unittest
4+
from test import support
35
from test.support import import_helper
6+
from test.support.script_helper import assert_python_failure
47

58
_testlimitedcapi = import_helper.import_module('_testlimitedcapi')
69
from _testcapi import PY_SSIZE_T_MIN, PY_SSIZE_T_MAX
@@ -172,6 +175,26 @@ def test_resize(self):
172175
# CRASHES resize(object(), 0)
173176
# CRASHES resize(NULL, 0)
174177

178+
@unittest.skipUnless(support.Py_DEBUG, 'need debug build (Py_DEBUG)')
179+
def test_detect_overflow(self):
180+
# Test detection of buffer overflow
181+
size = 123 # bytes
182+
overflow = 1 # bytes
183+
code = textwrap.dedent(f'''
184+
from test.support import SuppressCrashReport
185+
import _testcapi
186+
187+
size = {size}
188+
overflow = {overflow}
189+
with SuppressCrashReport():
190+
# Trigger a buffer overflow in a new bytearray
191+
ba = _testcapi.bytearray_overflow(size, overflow)
192+
ba = None
193+
''')
194+
proc = assert_python_failure('-c', code)
195+
self.assertIn(b'Buffer overflow detected in bytearray object', proc.err)
196+
self.assertIn(f'at position {size}'.encode(), proc.err)
197+
175198

176199
if __name__ == "__main__":
177200
unittest.main()

‎Lib/test/test_capi/test_bytes.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,26 @@ def test_join(self):
317317
with self.assertRaises(SystemError):
318318
bytes_join(b'', NULL)
319319

320+
@unittest.skipUnless(support.Py_DEBUG, 'need debug build (Py_DEBUG)')
321+
def test_detect_overflow(self):
322+
# Test detection of buffer overflow
323+
size = 123 # bytes
324+
overflow = 1 # bytes
325+
code = textwrap.dedent(f'''
326+
from test.support import SuppressCrashReport
327+
import _testcapi
328+
329+
size = {size}
330+
overflow = {overflow}
331+
with SuppressCrashReport():
332+
# Trigger a buffer overflow in a new bytes
333+
ba = _testcapi.bytes_overflow(size, overflow)
334+
ba = None
335+
''')
336+
proc = assert_python_failure('-c', code)
337+
self.assertIn(b'Buffer overflow detected in bytes object', proc.err)
338+
self.assertIn(f'at position {size}'.encode(), proc.err)
339+
320340

321341
def get_data_canary(writer):
322342
size = writer.get_size() + 1

‎Misc/NEWS.d/next/Core_and_Builtins/2026-09-14-23-16-26.gh-issue-156939.oZqlSt.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
When Python is built in debug mode, :class:`bytes` and :class:`bytearray`
2+
destructors now check if the trailing null byte has been overridden to detect
3+
buffer overflow. Patch by Victor Stinner.

‎Modules/_testcapi/bytes.c‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,13 +528,54 @@ test_byteswriter_ptr(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args))
528528
}
529529

530530

531+
static PyObject *
532+
bytes_overflow(PyObject *Py_UNUSED(module), PyObject *args)
533+
{
534+
Py_ssize_t alloc, overflow = 1;
535+
if (!PyArg_ParseTuple(args, "n|n", &alloc, &overflow))
536+
return NULL;
537+
538+
PyObject *bytes = PyObject_CallFunction((PyObject*)&PyBytes_Type, "n", alloc);
539+
if (bytes == NULL) {
540+
return NULL;
541+
}
542+
543+
char *data = PyBytes_AS_STRING(bytes);
544+
Py_ssize_t size = PyBytes_GET_SIZE(bytes);
545+
memset(data, 'x', size);
546+
memset(data + size, '#', overflow); // Buffer overflow!
547+
return bytes;
548+
}
549+
550+
551+
static PyObject *
552+
bytearray_overflow(PyObject *Py_UNUSED(module), PyObject *args)
553+
{
554+
Py_ssize_t alloc, overflow = 1;
555+
if (!PyArg_ParseTuple(args, "n|n", &alloc, &overflow))
556+
return NULL;
557+
558+
PyObject *bytearray = PyObject_CallFunction((PyObject*)&PyByteArray_Type, "n", alloc);
559+
if (bytearray == NULL) {
560+
return NULL;
561+
}
562+
563+
char *data = PyByteArray_AS_STRING(bytearray);
564+
Py_ssize_t size = PyByteArray_GET_SIZE(bytearray);
565+
memset(data + size, '#', overflow); // Buffer overflow!
566+
return bytearray;
567+
}
568+
569+
531570
static PyMethodDef test_methods[] = {
532571
{"bytes_resize", bytes_resize, METH_VARARGS},
533572
{"bytes_join", bytes_join, METH_VARARGS},
534573
{"byteswriter_abc", byteswriter_abc, METH_NOARGS},
535574
{"byteswriter_resize", byteswriter_resize, METH_NOARGS},
536575
{"byteswriter_highlevel", byteswriter_highlevel, METH_NOARGS},
537576
{"test_byteswriter_ptr", test_byteswriter_ptr, METH_NOARGS},
577+
{"bytes_overflow", bytes_overflow, METH_VARARGS},
578+
{"bytearray_overflow", bytearray_overflow, METH_VARARGS},
538579
{NULL},
539580
};
540581

‎Modules/_testcapi/mem.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,13 +448,15 @@ test_pyobject_new(PyObject *self, PyObject *Py_UNUSED(ignored))
448448
if (obj == NULL) {
449449
goto alloc_failed;
450450
}
451+
memset(PyBytes_AS_STRING(obj), 0, 3 + 1); // +1 for the null byte
451452
Py_DECREF(obj);
452453

453454
// PyObject_NEW_VAR()
454455
obj = PyObject_NEW_VAR(PyObject, var_type, 3);
455456
if (obj == NULL) {
456457
goto alloc_failed;
457458
}
459+
memset(PyBytes_AS_STRING(obj), 0, 3 + 1); // +1 for the null byte
458460
Py_DECREF(obj);
459461

460462
Py_RETURN_NONE;

‎Objects/bytearrayobject.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1272,6 +1272,12 @@ static void
12721272
bytearray_dealloc(PyObject *op)
12731273
{
12741274
PyByteArrayObject *self = _PyByteArray_CAST(op);
1275+
#ifdef Py_DEBUG
1276+
if (self->ob_bytes_object != NULL) {
1277+
_PyBytes_CheckOverflow(self->ob_bytes_object, op, "bytearray");
1278+
}
1279+
#endif
1280+
12751281
if (self->ob_exports > 0) {
12761282
PyErr_SetString(PyExc_SystemError,
12771283
"deallocated bytearray object has exported buffers");

‎Objects/bytesobject.c‎

Lines changed: 66 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3191,12 +3191,42 @@ bytes_iteritem(PyObject *obj, Py_ssize_t index)
31913191
return (_PyObjectIndexPair) { .object = l, .index = index + 1 };
31923192
}
31933193

3194+
#ifdef Py_DEBUG
3195+
void
3196+
_PyBytes_CheckOverflow(PyObject *self, void *addr, const char *type_name)
3197+
{
3198+
// Make sure that the trailing null byte was not modified
3199+
char *data = PyBytes_AS_STRING(self);
3200+
Py_ssize_t size = PyBytes_GET_SIZE(self);
3201+
if (data[size] != '\0') {
3202+
_Py_FatalErrorFormat(__func__,
3203+
"Buffer overflow detected in %s object %p "
3204+
"at position %zd",
3205+
type_name, addr, size);
3206+
}
3207+
}
3208+
3209+
3210+
static void
3211+
bytes_dealloc(PyObject *op)
3212+
{
3213+
PyBytesObject *self = _PyBytes_CAST(op);
3214+
_PyBytes_CheckOverflow(op, op, "bytes");
3215+
Py_TYPE(self)->tp_free((PyObject *)self);
3216+
}
3217+
#endif
3218+
3219+
31943220
PyTypeObject PyBytes_Type = {
31953221
PyVarObject_HEAD_INIT(&PyType_Type, 0)
31963222
"bytes",
31973223
PyBytesObject_SIZE,
31983224
sizeof(char),
3225+
#ifdef Py_DEBUG
3226+
bytes_dealloc, /* tp_dealloc */
3227+
#else
31993228
0, /* tp_dealloc */
3229+
#endif
32003230
0, /* tp_vectorcall_offset */
32013231
0, /* tp_getattr */
32023232
0, /* tp_setattr */
@@ -3665,6 +3695,18 @@ byteswriter_write_canary_byte(PyBytesWriter *writer)
36653695
unsigned char *data = (unsigned char*)byteswriter_data(writer);
36663696
data[writer->size] = PyBytesWriter_CANARY_BYTE;
36673697
}
3698+
3699+
3700+
static void
3701+
byteswriter_reset_trailing_byte(PyBytesWriter *writer)
3702+
{
3703+
// PyBytesWriter writes non-zero canary byte as the last byte.
3704+
// bytes/bytearray expects the last byte to be a null byte.
3705+
// Reset the last byte to null for bytes/bytearray.
3706+
Py_ssize_t allocated = byteswriter_allocated(writer);
3707+
char *data = byteswriter_data(writer);
3708+
data[allocated] = '\0';
3709+
}
36683710
#endif
36693711

36703712

@@ -3814,6 +3856,9 @@ PyBytesWriter_Discard(PyBytesWriter *writer)
38143856

38153857
#ifdef Py_DEBUG
38163858
byteswriter_check_canary_byte(writer);
3859+
if (writer->obj != NULL) {
3860+
byteswriter_reset_trailing_byte(writer);
3861+
}
38173862
#endif
38183863

38193864
Py_XDECREF(writer->obj);
@@ -3838,23 +3883,19 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size)
38383883
}
38393884

38403885
#ifdef Py_DEBUG
3841-
// Check for buffer overflow
38423886
byteswriter_check_canary_byte(writer);
3843-
3844-
if (writer->obj != NULL) {
3845-
// byteswriter_write_canary_byte() can override the trailing NUL byte.
3846-
// So reset the trailing NUL byte to NUL.
3847-
Py_ssize_t allocated = byteswriter_allocated(writer);
3848-
char *data = byteswriter_data(writer);
3849-
data[allocated] = '\0';
3850-
}
38513887
#endif
38523888

38533889
PyObject *result;
38543890
if (size == 0) {
38553891
result = bytes_get_empty();
38563892
}
38573893
else if (writer->obj != NULL) {
3894+
// Truncate the bytes/bytearray object if needed
3895+
#ifdef Py_DEBUG
3896+
byteswriter_reset_trailing_byte(writer);
3897+
#endif
3898+
38583899
if (writer->use_bytearray) {
38593900
if (size != PyByteArray_GET_SIZE(writer->obj)) {
38603901
if (PyByteArray_Resize(writer->obj, size)) {
@@ -3868,25 +3909,28 @@ PyBytesWriter_FinishWithSize(PyBytesWriter *writer, Py_ssize_t size)
38683909
goto error;
38693910
}
38703911
}
3912+
3913+
if (size == 1) {
3914+
// Get the single byte singleton
3915+
unsigned char ch = PyBytes_AS_STRING(writer->obj)[0];
3916+
PyObject *op = (PyObject*)CHARACTER(ch);
3917+
assert(_Py_IsImmortal(op));
3918+
Py_SETREF(writer->obj, op);
3919+
}
38713920
}
38723921

38733922
result = writer->obj;
38743923
writer->obj = NULL;
3875-
3876-
if (size == 1 && !writer->use_bytearray) {
3877-
// Get the single byte singleton
3878-
unsigned char ch = PyBytes_AS_STRING(result)[0];
3879-
PyObject *op = (PyObject*)CHARACTER(ch);
3880-
assert(_Py_IsImmortal(op));
3881-
Py_SETREF(result, op);
3882-
}
3883-
}
3884-
else if (writer->use_bytearray) {
3885-
result = PyByteArray_FromStringAndSize(writer->small_buffer, size);
38863924
}
38873925
else {
3888-
// The function returns single byte singleton if size equals 1
3889-
result = PyBytes_FromStringAndSize(writer->small_buffer, size);
3926+
// Create an object from the small buffer
3927+
if (writer->use_bytearray) {
3928+
result = PyByteArray_FromStringAndSize(writer->small_buffer, size);
3929+
}
3930+
else {
3931+
// The function returns single byte singleton if size equals 1
3932+
result = PyBytes_FromStringAndSize(writer->small_buffer, size);
3933+
}
38903934
}
38913935

38923936
#ifdef Py_DEBUG

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL