Skip to content

Python upb: empty extended slice in del repeated_field[start:stop:step] performs an out-of-bounds memmove #29672

Description

@Asmatan

Summary

Deleting an empty extended slice from a repeated field performs an out-of-bounds memmove
and crashes the process, instead of being a no-op as it is for a Python list.

from google.protobuf import descriptor_pb2
f = descriptor_pb2.FileDescriptorProto()
del f.dependency[::2]        # empty repeated field; a no-op in Python

Observed on protobuf==7.36.1 (upb runtime, google._upb._message):

Windows fatal exception: access violation
exit code 3221225477 (0xC0000005)

The pure-Python runtime and CPython list both treat this as a no-op.

Root cause

PyUpb_RepeatedContainer_DeleteSubscript() (python/repeated.c) assumes at least one
element is selected in the step > 1 branch:

if (step > 1) {
  src = start + 1;                 // assumes count > 0
  for (Py_ssize_t i = 1; i < count; i++, dst += step - 1, src += step) {
    upb_Array_Move(arr, dst, src, step);
  }
} else {
  src = start + count;
}

size_t tail = upb_Array_Size(arr) - src;   // underflows when src > size
size_t new_size = dst + tail;
assert(new_size == upb_Array_Size(arr) - count);   // compiled out in release builds
upb_Array_Move(arr, dst, src, tail);               // memmove(..., SIZE_MAX << lg2)

PySlice_AdjustIndices() returns count == 0 with start == len(array) for del x[len:len:2],
del x[::2] on an empty array, and del x[k:k:2] when k == len. The step > 1 branch then
sets src = start + 1 > size, so tail underflows to SIZE_MAX, and upb_Array_Move()
(upb/message/array.c) issues a memmove of ~2^64 bytes.

When start < len(array) the same branch instead silently drops one element, e.g.
del x[0:0:2] on 5 elements leaves 4, rather than being a no-op.

Reproduction matrix (protobuf 7.36.1, upb runtime)

input CPython list pure-Python runtime upb runtime
del x[::2], len 0 no-op no-op crash 0xC0000005
del x[5:5:2], len 5 no-op no-op crash 0xC0000005
del x[3:3:-2], len 5 no-op no-op crash 0xC0000005
del x[0:0:2], len 5 no-op no-op drops one element (5→4)
del x[1:5:2], len 5 (non-empty) [0,2,4] [0,2,4] [0,2,4] (correct)
del x[3:3:1], len 5 (step 1) no-op no-op no-op (correct)

Expected behavior

Deleting an empty slice should be a no-op, matching list and the pure-Python runtime.
python/minimal_test.py:test_repeated_field_slice_delete asserts exactly this list-parity
contract, but only for non-empty ranges.

Proposed fix

 static int PyUpb_RepeatedContainer_DeleteSubscript(upb_Array* arr,
                                                    Py_ssize_t idx,
                                                    Py_ssize_t count,
                                                    Py_ssize_t step) {
+  // An empty slice deletes nothing.  Without this, the step>1 branch below
+  // sets src = start + 1, which can exceed the array size and make
+  // `tail = upb_Array_Size(arr) - src` underflow size_t, turning the
+  // subsequent upb_Array_Move() into an out-of-bounds memmove.
+  if (count == 0) return 0;
+
   // Normalize direction: deletion is order-independent.
   Py_ssize_t start = idx;

and add empty-range cases to test_repeated_field_slice_delete:

    test_slice(5, 5, 2)
    test_slice(0, 0, 2)
    test_slice(2, 2, 2)
    test_slice(20, 20, 2)

Notes

This is reachable through the in-memory container API, which SECURITY.md lists outside the
CVE threat model, so I am filing it here as a public issue rather than a private security
report, per the policy's guidance for out-of-threat-model issues. The consequence is a heap
out-of-bounds write, and the trigger is a valid list operation rather than an invalid
argument.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions