Single-valued properties accessed with foreach_get/set check for 1 less value than is read/written #109024

New Issue

Thomas Barlow · 2023-06-15T18:38:10+02:00

Thomas Barlow commented

2023-06-15 18:38:10 +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: version: 2.79, 2.93.9, 3.5.1, 4.0.0 Alpha, branch: main, commit date: 2023-06-03 14:21, hash: ab6860f3de0f
Worked:

Short description of error
Most properties, where each property is a single value, accept a list/array that is one element too short when accessed with foreach_get and foreach_set, potentially reading/writing uniitialised memory if the list/array is one element short.

This is not the case for properties that contain multiple values and would be flattened by foreach_get/set, such as properties that are usually accessed as a Vector, or for 'FLOAT' and 'INT' type attributes.

Single-valued properties I've checked:

Key.key_blocks, e.g. my_key.key_blocks.foreach_get("value", my_list),
Object.vertex_groups e.g. my_object.vertex_groups.foreach_get("index", my_list),
Mesh.materials e.g. my_mesh.materials.foreach_get("roughness", my_list),
Object.material_slots e.g. my_obj.material_slots.foreach_get("link", my_list)
Mesh.color_attributes e.g. my_mesh.color_attributes.foreach_get("is_internal", my_list)
bpy.data.objects e.g. bpy.data.objects.foreach_get("add_rest_position_attribute", my_list)
MeshEdge.crease e.g. my_mesh.edges.foreach_get("crease", my_list)
'BOOLEAN' and 'INT8' type attributes, e.g. my_mesh.attributes['bool_or_int8_attribute'].data.foreach_get("value", my_list)

Mesh.materials is extra broken because it accept lists/arrays that are shorter yet again by the number of None materials e.g. If a mesh has 5 materials and no materials are None, a 4 length list is accepted. If a mesh has 5 materials and 2 materials are None, a 2 length list is accepted.

In the examples using Key.key_blocks, I've used the value property, but these issues also apply to the other properties usable with foreach_get/set (slider_min, slider_max, frame, mute and interpolation (interpolation is not a good test because it is an enum which is affected by #92621)).

Exact steps for others to reproduce the error
A .blend (authored in 4.0.0a) is attached that contains a default cube with 4 shape keys and the two scripts below copied into two Text Editors.

To show that supplying a list that is one value too short doesn't raise an exception:

Select a Mesh Object as active
Create shape keys until it has at least 2 (only 1 will not work due to an early return in C code when foreach_get is given an empty list)
Open the Python Console and run:
1. key_blocks = C.object.data.shape_keys.key_blocks
2. my_list = [None]*(len(key_blocks) - 1)
3. key_blocks.foreach_get("value", my_list)
Observe that no exception was raised due to the list being too short

To show that supplying a list that is two values too short does raise an exception as expected:

Open the System Console
Select a Mesh Object as active
Create shape keys until it has at least 3 (only 2 will not work due to an early return in C code when foreach_get is given an empty list)
Open the Python Console and run:
1. key_blocks = C.object.data.shape_keys.key_blocks
2. my_list = [None]*(len(key_blocks) - 2)
3. key_blocks.foreach_get("value", my_list)
Observe that an exception was raised due to the list being too short
Observe that the message printed in the System Console specifies that more elements was expected.

This script can be run from the Text editor and goes over a number of foreach_get cases with lists/arrays that are too short

import bpy
import numpy as np

# Ensure the active object has at least 3 (including the 'Basis') shape keys
key_blocks = bpy.context.object.data.shape_keys.key_blocks

num_shape_keys = len(key_blocks)
assert num_shape_keys >= 3

my_list_one_short = [None] * (num_shape_keys - 1)
# Using 100.0 as a starting value so it's easy to tell what's changed.
# Single precision can be used as a buffer because it matches the internal C
# type of the property
arr_single = np.full(num_shape_keys, 100.0, dtype=np.single)
arr_single_one_short_view = arr_single[:num_shape_keys - 1]
# Double precision cannot be used as a buffer, so will be used as a sequence
arr_double = np.full(num_shape_keys, 100.0, dtype=np.double)
arr_double_one_short_view = arr_double[:num_shape_keys - 1]

# All of these should fail because they're too short
key_blocks.foreach_get("value", my_list_one_short)
key_blocks.foreach_get("value", arr_single_one_short_view)
key_blocks.foreach_get("value", arr_double_one_short_view)

print("my_list_one_short")
print(my_list_one_short)
print("arr_single")
# Observe that even the last value not in arr_single_one_short_view has been
# overwritten. This could have been uninitialised memory if we hadn't made
# arr_single long enough!
print(arr_single)
# It's not clear whether a 4th value somewhere else in memory has been written
# to.
print("arr_double")
print(arr_double)

my_list_two_short = [None] * (num_shape_keys - 2)
# List must not be zero-length, otherwise foreach_get code returns early
# without doing anything
assert len(my_list_two_short) > 0

# An expected RuntimeError will be raised and an error will be printed to
# the system console, saying:
# "Error: Array length mismatch (got {num_shape_keys - 2}, expected more)"
key_blocks.foreach_get("value", my_list_two_short)
raise AssertionError("Unreachable")

This script can be run from the Text Editor and goes over a number of foreach_set cases with lists/arrays that are too short

import bpy
import numpy as np

# Ensure the active object has at least 3 (including the 'Basis') shape keys
key_blocks = bpy.context.object.data.shape_keys.key_blocks

num_shape_keys = len(key_blocks)
assert num_shape_keys >= 3

# Reset values to zero
for kb in key_blocks:
    kb.value = 0.0

print("Starting values")
print([kb. value for kb in key_blocks])

my_list_one_short = [0.4] * (num_shape_keys - 1)

key_blocks.foreach_set("value", my_list_one_short)
print("Values set from my_list_one_short")
# Observe that the last value may be a random number read from memory.
# In some cases, the last value will instead be zero, because the value read
# from memory will be outside the range of the shape key's slider_min and
# slider_max attributes.
print([kb. value for kb in key_blocks])

# Reset values to zero
for kb in key_blocks:
    kb.value = 0.0

arr_single = np.full(num_shape_keys, 0.5, dtype=np.single)
arr_single[-1] = 0.55
arr_single_one_short_view = arr_single[:num_shape_keys - 1]

key_blocks.foreach_set("value", arr_single_one_short_view)
print("Values set from arr_single_one_short_view")
# Observe that the last value was set to 0.55, because foreach_set code
# actually read beyond the end of the view.
print([kb. value for kb in key_blocks])

# Reset values to zero
for kb in key_blocks:
    kb.value = 0.0

arr_double = np.full(num_shape_keys, 0.6, dtype=np.double)
arr_double_one_short_view = arr_double[:num_shape_keys - 1]

key_blocks.foreach_set("value", arr_double_one_short_view)
print("Values set from arr_double_one_short_view")
# Observe that the last value may be a random number read from memory.
# In some cases, the last value will instead be zero, because the value read
# from memory will be outside the range of the shape key's slider_min and
# slider_max attributes.
print([kb. value for kb in key_blocks])

my_list_two_short = [0.3] * (num_shape_keys - 2)
# List must not be zero-length, otherwise foreach_set code returns early
# without doing anything
assert len(my_list_two_short) > 0

# An expected RuntimeError will be raised and an error will be printed to
# the system console, saying:
# "Error: Array length mismatch (got {num_shape_keys - 2}, expected more)"
key_blocks.foreach_get("value", my_list_two_short)
raise AssertionError("Unreachable")

**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: version: 2.79, 2.93.9, 3.5.1, 4.0.0 Alpha, branch: main, commit date: 2023-06-03 14:21, hash: `ab6860f3de0f` Worked: **Short description of error** Most properties, where each property is a single value, accept a list/array that is one element too short when accessed with `foreach_get` and `foreach_set`, potentially reading/writing uniitialised memory if the list/array is one element short. This is not the case for properties that contain multiple values and would be flattened by foreach_get/set, such as properties that are usually accessed as a Vector, or for 'FLOAT' and 'INT' type attributes. Single-valued properties I've checked: - `Key.key_blocks`, e.g. `my_key.key_blocks.foreach_get("value", my_list)`, - `Object.vertex_groups` e.g. `my_object.vertex_groups.foreach_get("index", my_list)`, - `Mesh.materials` e.g. `my_mesh.materials.foreach_get("roughness", my_list)`, - `Object.material_slots` e.g. `my_obj.material_slots.foreach_get("link", my_list)` - `Mesh.color_attributes` e.g. `my_mesh.color_attributes.foreach_get("is_internal", my_list)` - `bpy.data.objects` e.g. `bpy.data.objects.foreach_get("add_rest_position_attribute", my_list)` - `MeshEdge.crease` e.g. `my_mesh.edges.foreach_get("crease", my_list)` - 'BOOLEAN' and 'INT8' type attributes, e.g. `my_mesh.attributes['bool_or_int8_attribute'].data.foreach_get("value", my_list)` `Mesh.materials` is extra broken because it accept lists/arrays that are shorter yet again by the number of `None` materials e.g. If a mesh has 5 materials and no materials are `None`, a 4 length list is accepted. If a mesh has 5 materials and 2 materials are `None`, a 2 length list is accepted. In the examples using `Key.key_blocks`, I've used the `value` property, but these issues also apply to the other properties usable with foreach_get/set (`slider_min`, `slider_max`, `frame`, `mute` and `interpolation` (`interpolation` is not a good test because it is an enum which is affected by #92621)). **Exact steps for others to reproduce the error** A .blend (authored in 4.0.0a) is attached that contains a default cube with 4 shape keys and the two scripts below copied into two Text Editors. To show that supplying a list that is one value too short doesn't raise an exception: 1) Select a Mesh Object as active 1) Create shape keys until it has at least 2 (only 1 will not work due to an early return in C code when foreach_get is given an empty list) 1) Open the Python Console and run: 1) `key_blocks = C.object.data.shape_keys.key_blocks` 1) `my_list = [None]*(len(key_blocks) - 1)` 1) `key_blocks.foreach_get("value", my_list)` 1) Observe that no exception was raised due to the list being too short To show that supplying a list that is two values too short does raise an exception as expected: 1) Open the System Console 1) Select a Mesh Object as active 1) Create shape keys until it has at least 3 (only 2 will not work due to an early return in C code when foreach_get is given an empty list) 1) Open the Python Console and run: 1) `key_blocks = C.object.data.shape_keys.key_blocks` 1) `my_list = [None]*(len(key_blocks) - 2)` 1) `key_blocks.foreach_get("value", my_list)` 1) Observe that an exception was raised due to the list being too short 1) Observe that the message printed in the System Console specifies that more elements was expected. This script can be run from the Text editor and goes over a number of `foreach_get` cases with lists/arrays that are too short ```py import bpy import numpy as np # Ensure the active object has at least 3 (including the 'Basis') shape keys key_blocks = bpy.context.object.data.shape_keys.key_blocks num_shape_keys = len(key_blocks) assert num_shape_keys >= 3 my_list_one_short = [None] * (num_shape_keys - 1) # Using 100.0 as a starting value so it's easy to tell what's changed. # Single precision can be used as a buffer because it matches the internal C # type of the property arr_single = np.full(num_shape_keys, 100.0, dtype=np.single) arr_single_one_short_view = arr_single[:num_shape_keys - 1] # Double precision cannot be used as a buffer, so will be used as a sequence arr_double = np.full(num_shape_keys, 100.0, dtype=np.double) arr_double_one_short_view = arr_double[:num_shape_keys - 1] # All of these should fail because they're too short key_blocks.foreach_get("value", my_list_one_short) key_blocks.foreach_get("value", arr_single_one_short_view) key_blocks.foreach_get("value", arr_double_one_short_view) print("my_list_one_short") print(my_list_one_short) print("arr_single") # Observe that even the last value not in arr_single_one_short_view has been # overwritten. This could have been uninitialised memory if we hadn't made # arr_single long enough! print(arr_single) # It's not clear whether a 4th value somewhere else in memory has been written # to. print("arr_double") print(arr_double) my_list_two_short = [None] * (num_shape_keys - 2) # List must not be zero-length, otherwise foreach_get code returns early # without doing anything assert len(my_list_two_short) > 0 # An expected RuntimeError will be raised and an error will be printed to # the system console, saying: # "Error: Array length mismatch (got {num_shape_keys - 2}, expected more)" key_blocks.foreach_get("value", my_list_two_short) raise AssertionError("Unreachable") ``` This script can be run from the Text Editor and goes over a number of `foreach_set` cases with lists/arrays that are too short ```py import bpy import numpy as np # Ensure the active object has at least 3 (including the 'Basis') shape keys key_blocks = bpy.context.object.data.shape_keys.key_blocks num_shape_keys = len(key_blocks) assert num_shape_keys >= 3 # Reset values to zero for kb in key_blocks: kb.value = 0.0 print("Starting values") print([kb. value for kb in key_blocks]) my_list_one_short = [0.4] * (num_shape_keys - 1) key_blocks.foreach_set("value", my_list_one_short) print("Values set from my_list_one_short") # Observe that the last value may be a random number read from memory. # In some cases, the last value will instead be zero, because the value read # from memory will be outside the range of the shape key's slider_min and # slider_max attributes. print([kb. value for kb in key_blocks]) # Reset values to zero for kb in key_blocks: kb.value = 0.0 arr_single = np.full(num_shape_keys, 0.5, dtype=np.single) arr_single[-1] = 0.55 arr_single_one_short_view = arr_single[:num_shape_keys - 1] key_blocks.foreach_set("value", arr_single_one_short_view) print("Values set from arr_single_one_short_view") # Observe that the last value was set to 0.55, because foreach_set code # actually read beyond the end of the view. print([kb. value for kb in key_blocks]) # Reset values to zero for kb in key_blocks: kb.value = 0.0 arr_double = np.full(num_shape_keys, 0.6, dtype=np.double) arr_double_one_short_view = arr_double[:num_shape_keys - 1] key_blocks.foreach_set("value", arr_double_one_short_view) print("Values set from arr_double_one_short_view") # Observe that the last value may be a random number read from memory. # In some cases, the last value will instead be zero, because the value read # from memory will be outside the range of the shape key's slider_min and # slider_max attributes. print([kb. value for kb in key_blocks]) my_list_two_short = [0.3] * (num_shape_keys - 2) # List must not be zero-length, otherwise foreach_set code returns early # without doing anything assert len(my_list_two_short) > 0 # An expected RuntimeError will be raised and an error will be printed to # the system console, saying: # "Error: Array length mismatch (got {num_shape_keys - 2}, expected more)" key_blocks.foreach_get("value", my_list_two_short) raise AssertionError("Unreachable") ```

key_blocks_foreach_getset.blend

114 KiB

Thomas Barlow added the

labels 2023-06-15 18:38:11 +02:00

Thomas Barlow changed title from ~~Key.key_blocks (shape keys) foreach_get/set checks for 1 less value than is read/written~~ to Some foreach_get/set checks for 1 less value than is read/written

2023-06-15 19:51:42 +02:00

Thomas Barlow changed title from ~~Some foreach_get/set checks for 1 less value than is read/written~~ to Single-valued properties accessed with foreach_get/set check for 1 less value than is read/written

2023-06-15 20:13:48 +02:00

Jesse Yurkovich added

and removed

labels 2023-06-16 03:36:20 +02:00

Jesse Yurkovich commented

2023-06-16 03:37:11 +02:00

Confirmed. There appears to be an off-by-1 in rna_access.cc

diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc
index c98923db0f7..3f50e7843d0 100644
--- a/source/blender/makesrna/intern/rna_access.cc
+++ b/source/blender/makesrna/intern/rna_access.cc
@@ -4665,7 +4665,7 @@ static int rna_raw_access(ReportList *reports,

         /* editable check */
         if (!set || RNA_property_editable(&itemptr, iprop)) {
-          if (a + itemlen > in.len) {
+          if (a + itemlen >= in.len) {
             BKE_reportf(
                 reports, RPT_ERROR, "Array length mismatch (got %d, expected more)", in.len);
             err = 1;

Confirmed. There appears to be an off-by-1 in `rna_access.cc` ```Diff diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc index c98923db0f7..3f50e7843d0 100644 --- a/source/blender/makesrna/intern/rna_access.cc +++ b/source/blender/makesrna/intern/rna_access.cc @@ -4665,7 +4665,7 @@ static int rna_raw_access(ReportList *reports, /* editable check */ if (!set || RNA_property_editable(&itemptr, iprop)) { - if (a + itemlen > in.len) { + if (a + itemlen >= in.len) { BKE_reportf( reports, RPT_ERROR, "Array length mismatch (got %d, expected more)", in.len); err = 1; ```

Thomas Barlow referenced this issue

2023-06-16 05:57:25 +02:00

Missing max length checks accessing some properties with foreach_get/set #109033

Thomas Barlow referenced a pull request that will close this issue

2023-12-09 01:49:54 +01:00

Fix #109024: Off-by-1 in rna_access for non-array props without raw access #115967

Sign in to join this conversation.