Missing checks for #PyObject_GetBuffer success in bpy_prop_collection foreach_get/set and setting idprop arrays #107017

New Issue

Thomas Barlow · 2023-04-17T00:38:47+02:00

Thomas Barlow commented

2023-04-17 00:38:47 +02:00

System Information
Operating system: Windows-10-10.0.19045-SP0 64 Bits
Graphics card: NVIDIA GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 531.41

Blender Version
Broken: 3.5.0, 3.6.0 Alpha, branch: main, commit date: 2023-04-14 19:16, hash: b86fc55d3005
Worked:

Short description of error
idp_from_PySequence in idprop_py_api.c and foreach_getset in bpy_rna.c use PyObject_GetBuffer, but do not check that getting the buffer was successful. This can cause the code to interact with incompatible or uninitialized buffers.

Both of these cases request C-contiguous buffers as per their PyBUF_SIMPLE request flags. Passing in a PyObject * that represents a non-C-contiguous buffer will cause the PyObject_GetBuffer call to fail, but because it's failure is ignored, it can cause code to read from/write to the buffer as if it were C-contiguous, potentially accessing memory outside of the buffer.

Permalinks to the uses of PyObject_GetBuffer that are not checking for success:

		source/blender/python/generic/idprop_py_api.c
		Line 603 in bd86e719ab
	
				    PyObject_GetBuffer(ob, &buffer, PyBUF_SIMPLE | PyBUF_FORMAT);

		source/blender/python/intern/bpy_rna.c
		Line 5347 in bd86e719ab
	
				      PyObject_GetBuffer(seq, &buf, PyBUF_SIMPLE | PyBUF_FORMAT);

		source/blender/python/intern/bpy_rna.c
		Line 5403 in bd86e719ab
	
				      PyObject_GetBuffer(seq, &buf, PyBUF_SIMPLE | PyBUF_FORMAT);

Exact steps for others to reproduce the error
It can be a little confusing to set up a test case because the state of a Py_buffer from a failed PyObject_GetBuffer call is not well defined.
Numpy ndarray objects appear to check compatibility with the buffer request before filling in the buffer's fields, whereas memoryview objects appear to fill in the buffer's fields and then check compatibility. In both of these cases in the Blender source, there are additional checks against the buffer's fields to determine if the buffer is compatible for Blender to use, so I've used memoryview objects in the scripts below so that the additional checks against the buffer's fields also pass, causing Blender to erroneously use the incompatible buffer instead of falling back to treating the PyObject * as a sequence.

This script creates an idprop array with a non-contiguous buffer of every other element of an array, but the code ends up reading data from the buffer in a C-contiguous manner instead of falling back to accessing the data as a sequence.

import bpy
import numpy as np

array = np.arange(16, dtype=np.intc)
array_even = array[::2]

scene = bpy.context.scene
# BufferError occurs because the buffer is not C-contiguous,
# but is ignored, causing a SystemError if nothing else catches
# the error (e.g. the script ends immediately afterwards).
# Furthermore, the first 32 C-contiguous values get read into
# the idprop array instead of every other element as expected.
scene["test_idprop_array"] = memoryview(array_even)

try:
    # This picks up the uncaught BufferError from above
    print()
except BufferError as e:
    print("Uncaught BufferError:")
    print(e)
else:
    # Unreachable due to the issue
    print("No BufferError was raised")

expected = array_even.tolist()
actual = list(scene["test_idprop_array"])
# [0, 2, 4, 6, 8, 10, 12, 14]
print(f"Expected:\n{expected}")
# [0, 1, 2, 3, 4, 5, 6, 7]
print(f"Actual:\n{actual}")
assert actual == expected

This script attempts to read a mesh's vertex co into a non-contiguous memoryview. Because the memoryview as a buffer is incompatible and because memoryview objects are read-only when accessed as a sequence (they have PySequenceMethods.sq_item so pass PySequence_Check(), but have no PySequenceMethods.sq_ass_item), the buffer is expected to remain unchanged. The code ends up reading the vertex co into the first C-contiguous bytes of the buffer instead.

foreach_set has the same issue as foreach_get, but I have not prepared a script for it.

import bpy
import numpy as np

# A default cube or other mesh needs to be the active Object
me = bpy.context.object.data
num_verts = len(me.vertices)
num_co = num_verts * 3
# buffer must be single-precision float to be usable as a buffer
# for getting/setting vertex co
vert_co_dtype = np.single

# Create a C-contiguous array with twice as many values as
# required so that we can pass in a view of every other element
# as an non-C-contiguous array that fails the PyObject_GetBuffer
# request because the request flags only allow C-contiguous arrays
co_array = np.zeros(num_co * 2, dtype=vert_co_dtype)
assert np.all(co_array == 0)

# numpy ndarrays check compatibility with the buffer request before
# they fill in the buffer fields, so when the C foreach_getset code
# checks if the buffer's format is compatible, the format field will
# not have been initialized and the buffer will be considered
# incompatible, so the C code will fall back to using the array as a
# sequence, which is not useful for this test case.
#
# A memoryview of the array, however, fills in the buffer fields first,
# so it can pass the buffer format check.
mv_co_array_non_contig = memoryview(co_array[::2])

# This is expected to raise a TypeError because the buffer request
# fails due to the buffer not being C-contiguous. (note that memoryview
# objects currently always fail this buffer request due to #106696)
# The array should remain unchanged because even if the C code were
# to fall back to using the memoryview as a sequence, that would fail
# too, because memoryview objects are read-only when accessed as a
# sequence (in CPython, they only allow setting values when accessed
# as a buffer or a mapping)
try:
    me.vertices.foreach_get("co", mv_co_array_non_contig)
except TypeError as te:
    # BufferError: memoryview: underlying buffer is not C-contiguous
    # couldn't access the py sequence
    print("Got expected error TypeError (printed below) caused by BufferError (printed above)")
    print(te)
    print()

print(co_array)
# Should pass. Fails.
assert np.all(co_array == 0)

**System Information** Operating system: Windows-10-10.0.19045-SP0 64 Bits Graphics card: NVIDIA GeForce GTX 1070 Ti/PCIe/SSE2 NVIDIA Corporation 4.5.0 NVIDIA 531.41 **Blender Version** Broken: 3.5.0, 3.6.0 Alpha, branch: main, commit date: 2023-04-14 19:16, hash: `b86fc55d3005` Worked: **Short description of error** `idp_from_PySequence` in `idprop_py_api.c` and `foreach_getset` in `bpy_rna.c` use `PyObject_GetBuffer`, but do not check that getting the buffer was successful. This can cause the code to interact with incompatible or uninitialized buffers. Both of these cases request C-contiguous buffers as per their `PyBUF_SIMPLE` request flags. Passing in a `PyObject *` that represents a non-C-contiguous buffer will cause the `PyObject_GetBuffer` call to fail, but because it's failure is ignored, it can cause code to read from/write to the buffer as if it were C-contiguous, potentially accessing memory outside of the buffer. Permalinks to the uses of `PyObject_GetBuffer` that are not checking for success: https://projects.blender.org/blender/blender/src/commit/bd86e719ab917e455c728837b6de61d480d9b1df/source/blender/python/generic/idprop_py_api.c#L603 https://projects.blender.org/blender/blender/src/commit/bd86e719ab917e455c728837b6de61d480d9b1df/source/blender/python/intern/bpy_rna.c#L5347 https://projects.blender.org/blender/blender/src/commit/bd86e719ab917e455c728837b6de61d480d9b1df/source/blender/python/intern/bpy_rna.c#L5403 **Exact steps for others to reproduce the error** It can be a little confusing to set up a test case because the state of a `Py_buffer` from a failed `PyObject_GetBuffer` call is not well defined. Numpy `ndarray` objects appear to check compatibility with the buffer request before filling in the buffer's fields, whereas `memoryview` objects appear to fill in the buffer's fields and then check compatibility. In both of these cases in the Blender source, there are additional checks against the buffer's fields to determine if the buffer is compatible for Blender to use, so I've used `memoryview` objects in the scripts below so that the additional checks against the buffer's fields also pass, causing Blender to erroneously use the incompatible buffer instead of falling back to treating the `PyObject *` as a sequence. This script creates an idprop array with a non-contiguous buffer of every other element of an array, but the code ends up reading data from the buffer in a C-contiguous manner instead of falling back to accessing the data as a sequence. ```py import bpy import numpy as np array = np.arange(16, dtype=np.intc) array_even = array[::2] scene = bpy.context.scene # BufferError occurs because the buffer is not C-contiguous, # but is ignored, causing a SystemError if nothing else catches # the error (e.g. the script ends immediately afterwards). # Furthermore, the first 32 C-contiguous values get read into # the idprop array instead of every other element as expected. scene["test_idprop_array"] = memoryview(array_even) try: # This picks up the uncaught BufferError from above print() except BufferError as e: print("Uncaught BufferError:") print(e) else: # Unreachable due to the issue print("No BufferError was raised") expected = array_even.tolist() actual = list(scene["test_idprop_array"]) # [0, 2, 4, 6, 8, 10, 12, 14] print(f"Expected:\n{expected}") # [0, 1, 2, 3, 4, 5, 6, 7] print(f"Actual:\n{actual}") assert actual == expected ``` This script attempts to read a mesh's vertex co into a non-contiguous `memoryview`. Because the `memoryview` as a buffer is incompatible and because `memoryview` objects are read-only when accessed as a sequence (they have `PySequenceMethods.sq_item` so pass `PySequence_Check()`, but have no `PySequenceMethods.sq_ass_item`), the buffer is expected to remain unchanged. The code ends up reading the vertex co into the first C-contiguous bytes of the buffer instead. `foreach_set` has the same issue as `foreach_get`, but I have not prepared a script for it. ```py import bpy import numpy as np # A default cube or other mesh needs to be the active Object me = bpy.context.object.data num_verts = len(me.vertices) num_co = num_verts * 3 # buffer must be single-precision float to be usable as a buffer # for getting/setting vertex co vert_co_dtype = np.single # Create a C-contiguous array with twice as many values as # required so that we can pass in a view of every other element # as an non-C-contiguous array that fails the PyObject_GetBuffer # request because the request flags only allow C-contiguous arrays co_array = np.zeros(num_co * 2, dtype=vert_co_dtype) assert np.all(co_array == 0) # numpy ndarrays check compatibility with the buffer request before # they fill in the buffer fields, so when the C foreach_getset code # checks if the buffer's format is compatible, the format field will # not have been initialized and the buffer will be considered # incompatible, so the C code will fall back to using the array as a # sequence, which is not useful for this test case. # # A memoryview of the array, however, fills in the buffer fields first, # so it can pass the buffer format check. mv_co_array_non_contig = memoryview(co_array[::2]) # This is expected to raise a TypeError because the buffer request # fails due to the buffer not being C-contiguous. (note that memoryview # objects currently always fail this buffer request due to #106696) # The array should remain unchanged because even if the C code were # to fall back to using the memoryview as a sequence, that would fail # too, because memoryview objects are read-only when accessed as a # sequence (in CPython, they only allow setting values when accessed # as a buffer or a mapping) try: me.vertices.foreach_get("co", mv_co_array_non_contig) except TypeError as te: # BufferError: memoryview: underlying buffer is not C-contiguous # couldn't access the py sequence print("Got expected error TypeError (printed below) caused by BufferError (printed above)") print(te) print() print(co_array) # Should pass. Fails. assert np.all(co_array == 0) ```

Thomas Barlow added the

labels 2023-04-17 00:38:48 +02:00

Thomas Barlow referenced a pull request that will close this issue

2023-04-17 01:36:23 +02:00

Fix #107017: Missing checks for #PyObject_GetBuffer success #107019

Campbell Barton referenced this issue from a commit