protocolbuffers / protocolbuffers/protobuf

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

Open Beginner friendly
#29,672 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
72k
Forks
16.3k
Avg merge
1d 17h
Merged PRs (30d)
140

Description

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in python/repeated.c at PyUpb_RepeatedContainer_DeleteSubscript(), then read python/minimal_test.py:test_repeated_field_slice_delete. Verify empty extended-slice deletions match CPython lists and the pure-Python runtime without crashing or changing the container, and run the focused test.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, python
Domain
backend, testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
92/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.