From 45aa6a38747702d4ee7c11b5c121bf6b73b79e7f Mon Sep 17 00:00:00 2001 From: Stepan Neretin Date: Sun, 13 Apr 2025 18:36:00 +0700 Subject: [PATCH 1/6] Fix: Prevent crash in ctypes.CField when byte_size does not match type size (gh-132470) When creating a ctypes.CField with an incorrect byte_size (e.g., using byte_size=2 for ctypes.c_byte), the code would previously abort due to the failed assertion byte_size == info->size. This commit replaces the assertion with a proper error handling mechanism that raises a ValueError when byte_size does not match the expected type size. This prevents the crash and provides a more informative error message to the us --- Modules/_ctypes/cfield.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 056e6dfd883a56..f486cfb6a92f25 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -99,7 +99,12 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, "type of field %R must be a C type", name); goto error; } - assert(byte_size == info->size); + if (byte_size != info->size) { + PyErr_Format(PyExc_ValueError, + "byte size of field %R (%zd) does not match type size (%zd)", + name, byte_size, info->size); + goto error; + } Py_ssize_t bitfield_size = 0; Py_ssize_t bit_offset = 0; From 3377d9a746abf9921d2ae6239d6a45cfc98c2d08 Mon Sep 17 00:00:00 2001 From: Stepan Neretin Date: Sun, 13 Apr 2025 20:42:17 +0700 Subject: [PATCH 2/6] Add regression test for invalid byte_size in ctypes.CField Adds a test to ensure that creating a ctypes.CField with a byte_size that doesn't match the underlying C type size (e.g., 2 bytes for c_byte) correctly raises a ValueError. This test verifies the fix for gh-132470 and prevents future regressions where such mismatches could cause an abort. --- Lib/test/test_ctypes/test_struct_fields.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index 5c713247a0f418..5f4837501d0351 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -1,6 +1,6 @@ import unittest import sys -from ctypes import Structure, Union, sizeof, c_char, c_int, CField +from ctypes import Structure, Union, sizeof, c_byte, c_char, c_int, CField from ._support import Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin @@ -75,6 +75,19 @@ def __init_subclass__(cls, **kwargs): 'ctypes state is not initialized'): class Subclass(BrokenStructure): ... + def test_invalid_byte_size_raises(self): + with self.assertRaises(ValueError) as cm: + CField( + name="a", + type=c_byte, + byte_size=2, # Wrong size: c_byte is only 1 byte + byte_offset=2, + index=1, + _internal_use=True + ) + + self.assertIn("does not match type size", str(cm.exception)) + def test_max_field_size_gh126937(self): # Classes for big structs should be created successfully. # (But they most likely can't be instantiated.) From 4c558be894e923ed1c13bd6e5800121cab70fa03 Mon Sep 17 00:00:00 2001 From: Stepan Neretin Date: Sun, 13 Apr 2025 20:52:45 +0700 Subject: [PATCH 3/6] add news --- .../2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst diff --git a/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst b/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst new file mode 100644 index 00000000000000..f95b38c6549957 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst @@ -0,0 +1,14 @@ +Creating a ctypes.CField with a byte_size that does not match the actual +type size now raises a ValueError instead of crashing the interpreter. # +Please enter the relevant GitHub issue number here: # + +# # Uncomment one of these "section:" lines to specify which section # this +entry should go in in Misc/NEWS.d. # #.. section: Security #.. section: Core +and Builtins #.. section: Library #.. section: Documentation #.. section: +Tests #.. section: Build #.. section: Windows #.. section: macOS #.. +section: IDLE #.. section: Tools/Demos #.. section: C API + +# Write your Misc/NEWS.d entry below. It should be a simple ReST paragraph. +# Don't start with "- Issue #: " or "- gh-issue-: " or that sort of +stuff. +########################################################################### From b20b0d8724ae58530b91abdfc1f6ba6c441b4f25 Mon Sep 17 00:00:00 2001 From: Stepan Neretin Date: Sun, 13 Apr 2025 20:59:36 +0700 Subject: [PATCH 4/6] fix review test --- Lib/test/test_ctypes/test_struct_fields.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index 5f4837501d0351..e377902a592749 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -75,8 +75,8 @@ def __init_subclass__(cls, **kwargs): 'ctypes state is not initialized'): class Subclass(BrokenStructure): ... - def test_invalid_byte_size_raises(self): - with self.assertRaises(ValueError) as cm: + def test_invalid_byte_size_raises_gh132470(self): + with self.assertRaisesRegex(ValueError, r"does not match type size"): CField( name="a", type=c_byte, @@ -86,8 +86,6 @@ def test_invalid_byte_size_raises(self): _internal_use=True ) - self.assertIn("does not match type size", str(cm.exception)) - def test_max_field_size_gh126937(self): # Classes for big structs should be created successfully. # (But they most likely can't be instantiated.) From be003721c9c05b4e368e43b23894268af4beba02 Mon Sep 17 00:00:00 2001 From: dura0ok Date: Wed, 16 Apr 2025 15:18:15 +0700 Subject: [PATCH 5/6] Update news Co-authored-by: sobolevn --- ...025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst b/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst index f95b38c6549957..5a03908fb91f03 100644 --- a/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst +++ b/Misc/NEWS.d/next/C_API/2025-04-13-20-52-39.gh-issue-132470.UqBQjN.rst @@ -1,14 +1,2 @@ -Creating a ctypes.CField with a byte_size that does not match the actual -type size now raises a ValueError instead of crashing the interpreter. # -Please enter the relevant GitHub issue number here: # - -# # Uncomment one of these "section:" lines to specify which section # this -entry should go in in Misc/NEWS.d. # #.. section: Security #.. section: Core -and Builtins #.. section: Library #.. section: Documentation #.. section: -Tests #.. section: Build #.. section: Windows #.. section: macOS #.. -section: IDLE #.. section: Tools/Demos #.. section: C API - -# Write your Misc/NEWS.d entry below. It should be a simple ReST paragraph. -# Don't start with "- Issue #: " or "- gh-issue-: " or that sort of -stuff. -########################################################################### +Creating a :class:`ctypes.CField` with a *byte_size* that does not match the actual +type size now raises a :exc:`ValueError` instead of crashing the interpreter. From 1d5f7cbce002c2376ec79cd366d29dfe70297f04 Mon Sep 17 00:00:00 2001 From: Stepan Neretin Date: Wed, 16 Apr 2025 17:07:58 +0700 Subject: [PATCH 6/6] fix space --- Lib/test/test_ctypes/test_struct_fields.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index e377902a592749..b50bbcbb65c423 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -80,7 +80,7 @@ def test_invalid_byte_size_raises_gh132470(self): CField( name="a", type=c_byte, - byte_size=2, # Wrong size: c_byte is only 1 byte + byte_size=2, # Wrong size: c_byte is only 1 byte byte_offset=2, index=1, _internal_use=True