Issue #23985: Fix a possible buffer overrun when deleting a slice from the front of a bytearray and then appending some other bytes data.

Patch by Martin Panter.

3 files changed, 21 insertions, 6 deletions

diff --git a/Lib/test/test_bytes.py b/Lib/test/test_bytes.py
index e15807e..6b58e74 100644
--- a/Lib/test/test_bytes.py
+++ b/Lib/test/test_bytes.py
@@ -947,6 +947,22 @@ class ByteArrayTest(BaseBytesTest, unittest.TestCase):
         b.extend(range(100, 110))
         self.assertEqual(list(b), list(range(10, 110)))
 
+    def test_fifo_overrun(self):
+        # Test for issue #23985, a buffer overrun when implementing a FIFO
+        # Build Python in pydebug mode for best results.
+        b = bytearray(10)
+        b.pop()        # Defeat expanding buffer off-by-one quirk
+        del b[:1]      # Advance start pointer without reallocating
+        b += bytes(2)  # Append exactly the number of deleted bytes
+        del b          # Free memory buffer, allowing pydebug verification
+
+    def test_del_expand(self):
+        # Reducing the size should not expand the buffer (issue #23985)
+        b = bytearray(10)
+        size = sys.getsizeof(b)
+        del b[:1]
+        self.assertLessEqual(sys.getsizeof(b), size)
+
     def test_extended_set_del_slice(self):
         indices = (0, None, 1, 3, 19, 300, 1<<333, -1, -2, -31, -300)
         for start in indices:
diff --git a/Misc/NEWS b/Misc/NEWS
index 12b141d..45a0639 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #23985: Fix a possible buffer overrun when deleting a slice from
+  the front of a bytearray and then appending some other bytes data.
+
 - Issue #24102: Fixed exception type checking in standard error handlers.
 
 - Issue #20274: Remove ignored and erroneous "kwargs" parameters from three
diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c
index f5eb321..8629ab7 100644
--- a/Objects/bytearrayobject.c
+++ b/Objects/bytearrayobject.c
@@ -179,7 +179,7 @@ PyByteArray_Resize(PyObject *self, Py_ssize_t requested_size)
         return -1;
     }
 
-    if (size + logical_offset + 1 < alloc) {
+    if (size + logical_offset + 1 <= alloc) {
         /* Current buffer is large enough to host the requested size,
            decide on a strategy. */
         if (size < alloc / 2) {
@@ -298,11 +298,7 @@ bytearray_iconcat(PyByteArrayObject *self, PyObject *other)
         PyBuffer_Release(&vo);
         return PyErr_NoMemory();
     }
-    if (size < self->ob_alloc) {
-        Py_SIZE(self) = size;
-        PyByteArray_AS_STRING(self)[Py_SIZE(self)] = '\0'; /* Trailing null byte */
-    }
-    else if (PyByteArray_Resize((PyObject *)self, size) < 0) {
+    if (PyByteArray_Resize((PyObject *)self, size) < 0) {
         PyBuffer_Release(&vo);
         return NULL;
     }