Repository: python/cpython
Pull request: gh-124248: Fix crash in struct when processing 0p fields
Fixes gh-124248.

Adds special handling for the case when `code->size` is 0 before attempting to decrement `code->size` or read the first byte under `res`.

After this change, `0p` should behave like `0s`.

Problem statement:
SystemError/Assertion failure when processing struct with '0p' field
# Crash report

### What happened?

Using `struct` to process zero-width Pascal strings (`"0p"`) can lead to an assertion failure or `SystemError`.

Specifically:
* `struct.pack("<0p", b"")` leads to an assertion failure and seg fault on debug builds (tested with current `main` and 3.13 tip)
* `struct.unpack("<0p", b"")` raises an unexpected `SystemError` (tested with current `main`, 3.13 tip, non-debug builds of 3.8-3.12)


On current `main` (`8f82d9aa219`):
```shell
$ make clean && ./configure --with-pydebug && make -j
$ ./python -VV
Python 3.14.0a0 (heads/main:8f82d9aa219, Sep 19 2024, 11:08:04) [GCC 11.4.0]


$ ./python -c 'import struct; struct.unpack("<0p", b"")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
    import struct; struct.unpack("<0p", b"")
                   ~~~~~~~~~~~~~^^^^^^^^^^^^
SystemError: Negative size passed to PyBytes_FromStringAndSize


$ ./python -c 'import struct; struct.pack("<0p", b"")'
python: ./Modules/_struct.c:1991: s_pack_internal: Assertion `_Py_STATIC_CAST(Py_ssize_t, _Py_STATIC_CAST(unsigned char, (n))) == (n)' failed.
[1]    186971 IOT instruction (core dumped)  ./python -c 'import struct; struct.pack("<0p", b"")'
```
The same behavior is reproducible on the current 3.13 tip (`112b1704fa6`), and likely previous versions.

PR forthcoming.



### CPython versions tested on:

CPython main branch

### Operating systems tested on:

Linux

### Output from running 'python -VV' on the command line:

Python 3.14.0a0 (heads/main:8f82d9aa219, Sep 19 2024, 11:08:04) [GCC 11.4.0]

<!-- gh-linked-prs -->
### Linked PRs
* gh-124251
* gh-124277
* gh-124278
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: Update NEWS

Patch to review:
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
index 5508cc3eec85c8..bdbf8800cfd8f6 100644
--- a/Lib/test/test_struct.py
+++ b/Lib/test/test_struct.py
@@ -96,6 +96,13 @@ def test_new_features(self):
             ('10s', b'helloworld', b'helloworld', b'helloworld', 0),
             ('11s', b'helloworld', b'helloworld\0', b'helloworld\0', 1),
             ('20s', b'helloworld', b'helloworld'+10*b'\0', b'helloworld'+10*b'\0', 1),
+            ('0p', b'helloworld', b'', b'', 1),
+            ('1p', b'helloworld', b'\x00', b'\x00', 1),
+            ('2p', b'helloworld', b'\x01h', b'\x01h', 1),
+            ('10p', b'helloworld', b'\x09helloworl', b'\x09helloworl', 1),
+            ('11p', b'helloworld', b'\x0Ahelloworld', b'\x0Ahelloworld', 0),
+            ('12p', b'helloworld', b'\x0Ahelloworld\0', b'\x0Ahelloworld\0', 1),
+            ('20p', b'helloworld', b'\x0Ahelloworld'+9*b'\0', b'\x0Ahelloworld'+9*b'\0', 1),
             ('b', 7, b'\7', b'\7', 0),
             ('b', -7, b'\371', b'\371', 0),
             ('B', 7, b'\7', b'\7', 0),
@@ -339,6 +346,7 @@ def assertStructError(func, *args, **kwargs):
     def test_p_code(self):
         # Test p ("Pascal string") code.
         for code, input, expected, expectedback in [
+                ('0p', b'abc', b'',                b''),
                 ('p',  b'abc', b'\x00',            b''),
                 ('1p', b'abc', b'\x00',            b''),
                 ('2p', b'abc', b'\x01a',           b'a'),
@@ -580,6 +588,7 @@ def test__sizeof__(self):
         self.check_sizeof('187s', 1)
         self.check_sizeof('20p', 1)
         self.check_sizeof('0s', 1)
+        self.check_sizeof('0p', 1)
         self.check_sizeof('0c', 0)
 
     def test_boundary_error_message(self):
diff --git a/Misc/NEWS.d/next/Library/2024-09-19-11-47-39.gh-issue-124248.g7rufd.rst b/Misc/NEWS.d/next/Library/2024-09-19-11-47-39.gh-issue-124248.g7rufd.rst
new file mode 100644
index 00000000000000..1bd333f485a2ab
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-09-19-11-47-39.gh-issue-124248.g7rufd.rst
@@ -0,0 +1,2 @@
+Fixed potential crash when using :mod:`struct` to process zero-width
+'Pascal string' fields (``0p``).
diff --git a/Modules/_struct.c b/Modules/_struct.c
index f744193469e2dc..9857ed7eae9543 100644
--- a/Modules/_struct.c
+++ b/Modules/_struct.c
@@ -1669,9 +1669,15 @@ s_unpack_internal(PyStructObject *soself, const char *startfrom,
             if (e->format == 's') {
                 v = PyBytes_FromStringAndSize(res, code->size);
             } else if (e->format == 'p') {
-                Py_ssize_t n = *(unsigned char*)res;
-                if (n >= code->size)
-                    n = code->size - 1;
+                Py_ssize_t n;
+                if (code->size == 0) {
+                    n = 0;
+                } else {
+                    n = *(unsigned char*)res;
+                    if (n >= code->size) {
+                        n = code->size - 1;
+                    }
+                }
                 v = PyBytes_FromStringAndSize(res + 1, n);
             } else {
                 v = e->unpack(state, res, e);
@@ -1982,8 +1988,11 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset,
                     n = PyByteArray_GET_SIZE(v);
                     p = PyByteArray_AS_STRING(v);
                 }
-                if (n > (code->size - 1))
+                if (code->size == 0) {
+                    n = 0;
+                } else if (n > (code->size - 1)) {
                     n = code->size - 1;
+                }
                 if (n > 0)
                     memcpy(res + 1, p, n);
                 if (n > 255)


Code review comments:
diff_hunk: @@ -1669,9 +1669,15 @@ s_unpack_internal(PyStructObject *soself, const char *startfrom,
             if (e->format == 's') {
                 v = PyBytes_FromStringAndSize(res, code->size);
             } else if (e->format == 'p') {
-                Py_ssize_t n = *(unsigned char*)res;
-                if (n >= code->size)
-                    n = code->size - 1;
+                Py_ssize_t n;
+                if (code->size == 0) {
+                    n = 0;
+                } else {
path: Modules/_struct.c
text: @user1:
[PEP7](https://peps.python.org/pep-0007/) ;)
```suggestion
                }
                else {
```

@author:
Thanks!

diff_hunk: @@ -1982,8 +1988,11 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset,
                     n = PyByteArray_GET_SIZE(v);
                     p = PyByteArray_AS_STRING(v);
                 }
-                if (n > (code->size - 1))
+                if (code->size == 0) {
+                    n = 0;
+                } else if (n > (code->size - 1)) {
path: Modules/_struct.c
text: @user1:
ditto
```suggestion
                }
                else if (n > (code->size - 1)) {
```