Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion Lib/test/test_capi/test_unicode.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 23 additions & 0 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 */
};

Expand Down
15 changes: 15 additions & 0 deletions Objects/unicode_writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down
9 changes: 5 additions & 4 deletions Objects/unicodeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down
Loading