Skip to content

Check the buffer size of orthogonal selections - #787

Open
FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-oselection-buffer-size
Open

FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-oselection-buffer-size

Conversation

@FabianIsensee

Copy link
Copy Markdown
Contributor

get_oindex_numpy() and set_oindex_numpy() pass typesize * prod(selection lengths) to b2nd_get/set_orthogonal_selection() as the buffer size, while their callers allocate the NumPy buffer for the array's dtype. When the dtype's items are smaller than the typesize, c-blosc2 writes (get) or reads (set) past the end of that buffer.

That happens with valid files. c-blosc2 2.7 to 2.16 stored "|u1" as the dtype of arrays created without one, whatever the typesize (2.17 writes "|S{typesize}"), for example c-blosc2's own plugins/test_data/example_item_prices.b2nd (float32 items, typesize 4), which python-blosc2 opens as uint8:

  • blosc2.open("example_item_prices.b2nd").get_oselection_numpy([range(12), range(25), range(250)]) segfaults (4.14.1 and main). Under ASan: heap-buffer-overflow WRITE in copy_block_buffer_data() right after the 75000-byte result buffer (the selection is 300000 bytes).
  • oindex[...] get and set, take(..., axis=...) and integer-array indexing of n-D arrays (a[[0, 1]], through take(axis=0)) go through the same two functions and do the same. A one-item selection writes 4 bytes into a 1-byte buffer without crashing; a set reads past the value and writes those bytes into the file.
  • A damaged dtype string with an intact typesize (e.g. <f8 became <f2) does the same.

The other read paths already refuse these arrays: a[...], slices, np.asarray(a) raise RuntimeError: Error while getting the buffer (c-blosc2 gets the real buffer length there), and flat take() raises in get_sparse_numpy(), which checks view.len.

This PR checks the buffer's length against the selection at the start of both functions, before the index arrays are allocated, and raises ValueError: destination buffer is smaller than the requested orthogonal selection (75000 < 300000 bytes): its itemsize is 1, but the array's typesize is 4 ("source" for writes). Larger buffers are accepted as before. The size passed to c-blosc2 is unchanged, so this works with and without Blosc/c-blosc2#864 (which fixes the inverted buffer check in orthogonal_selection()). The check sits outside the lines that #770 restructures, so the two PRs merge in either order; it also merges cleanly with #779.

Tests, in the new tests/ndarray/test_oindex_typesize.py: example_day_month_temp.b2nd from c-blosc2's plugins/test_data (2.6 KB, copied into tests/data/legacy_u1_dtype/) read and written through oselection, oindex and a[[0, 1]]; the layout of example_item_prices.b2nd (141 KB, not copied) and a float64 array with the dtype string patched to "|u1" in a cframe; a damaged dtype string (<f8 -> <f2); too small, exact, larger and empty buffers passed to the extension methods directly; and valid arrays of several dtypes (bool, big-endian int32, S7, <U100 with 400-byte items, structured) through oindex. The legacy, damaged and buffer tests crash or fail on main; all pass with the fix. Full suite: same results as main (only the 2 b2view tests that need aiohttp fail locally). ASan build: no reports in these tests, the indexing test files, and 37 read/write calls on the example file.

Cost: about 0.05-0.15 µs per call, for a second buffer acquisition and the size product (in Python ints, which cannot wrap around): ~2-3 µs one-block selections get 2-5% slower, a 9 µs one 0.5-1.6%, take and writes do not change measurably (Ryzen 9 7950X, 1 thread, two runs of 6 alternated rounds, main built twice for the noise floor: within 1.1%). After #770 the check could move behind its PyObject_GetBuffer and compare buf.len with buffersize_ for free, and once the bundled c-blosc2 has #864, passing buf.len would let c-blosc2 do the check; I kept it separate here so the PRs do not depend on each other.

Not in this PR: these legacy arrays still cannot be read through NumPy (as before, every read path now raises instead of crashing; tobytes() works). If you want them to read, python-blosc2 could present a stored "|u1" with a typesize above 1 as S{typesize}, as c-blosc2 >= 2.17 writes such arrays and as python-blosc2 already shows arrays without a dtype. That changes .dtype for these files, so I left it out; happy to do it in a follow-up if you want it.

I used Claude Code (Anthropic) to trace the bug, write the fix and the tests, and run the measurements; I reviewed the change and the numbers.

🤖 Generated with Claude Code

get_oindex_numpy() and set_oindex_numpy() pass typesize * prod(selection
lengths) to the C orthogonal selection as the buffer size, but their
callers allocate the NumPy buffer for the array's dtype.  When the
dtype's items are smaller than the typesize, the C copy writes (get) or
reads (set) past the end of that buffer.  This happens with valid files:
c-blosc2 2.7 to 2.16 stored "|u1" as the dtype of arrays created without
one, whatever the typesize, e.g. c-blosc2's own
plugins/test_data/example_item_prices.b2nd (float32 items, typesize 4),
whose get_oselection_numpy() segfaults.  A damaged dtype string does the
same.  oindex, take(axis=...) and integer-array indexing of
multi-dimensional arrays go through the same two functions.

Check the length of the buffer against the selection at the start of
both functions, before the index arrays are allocated, and raise
ValueError when it is too small, as get_sparse_numpy() does (slices are
checked by c-blosc2, which gets their real buffer length).  This does
not depend on the buffer check of the C orthogonal selection, which
accepted too small buffers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DiPFzu8ziFQsxrJUkHCrzx

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant