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

gh-157710: Detect overflow in PyUnicodeWriter_Finish() by vstinner · Pull Request #157715 · python/cpython · GitHub

Repository navigation

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

Filter by extension

Filter by extension .c  (3) .py  (1) .rst  (1) All 3 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
20 changes: 19 additions & 1 deletion Lib/test/test_capi/test_unicode.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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

Back | FazBrowse Home | New Git URL