Repository navigation
Conversation
The memoryview branch copied p_buf->len bytes from the base pointer of PyMemoryView_GET_BUFFER, which only spans the payload when the view is C-contiguous. A reversed or strided view puts buf at a different offset and carries its own strides, so the copy ran off the end of the underlying buffer and the out-of-bounds bytes were spliced into the packed command: packing memoryview(bytearray(b'.' * 32))[::-1] returned 15 bytes of the next heap allocation, and a 256 MiB bytearray segfaulted. Acquiring the view with PyObject_GetBuffer under PyBUF_SIMPLE rejects non-contiguous input with the same BufferError that Reader.feed already raises for it, and leaves contiguous memoryviews unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
pack_command copies p_buf->len bytes from the base pointer PyMemoryView_GET_BUFFER returns, which only spans the payload when the view is C-contiguous; a reversed or strided memoryview puts buf at a different offset and carries its own strides, so the copy runs off the end of the underlying buffer and the out-of-bounds bytes end up in the packed command. Packing
memoryview(bytearray(b'.' * 32))[::-1]returns 15 bytes belonging to the next heap allocation, and a 256 MiB bytearray segfaults. Acquiring the view with PyObject_GetBuffer under PyBUF_SIMPLE rejects non-contiguous input with the same BufferError that Reader.feed already raises for it and leaves contiguous memoryviews unchanged; the added test fails before the change, and the suite passes on CPython 3.11 and PyPy 3.11 with and without NDEBUG.Note
High Risk
Fixes a memory-safety bug (OOB read / possible segfault and leaking adjacent heap bytes into packed commands) in command serialization.
Overview
Fixes an out-of-bounds read when
pack_commandpacks a non-contiguousmemoryview(e.g. reversed or strided). The old path usedPyMemoryView_GET_BUFFERand copiedlenbytes frombufas if the view were C-contiguous, which could read past the underlying buffer and put garbage (or crash) into the serialized Redis command.Memoryview arguments are now acquired via
PyObject_GetBuffer(..., PyBUF_SIMPLE)with error handling andPyBuffer_Release, so non-contiguous views raiseBufferErrorinstead of being packed incorrectly. Contiguous memoryviews behave as before.A regression test packs a reversed
memoryviewand expectsBufferError.Reviewed by Cursor Bugbot for commit 216061c. Bugbot is set up for automated code reviews on this repo. Configure here.