From eeb21fc8c316bf761e2eb69ab8eefea366f35803 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Thu, 23 Mar 2023 13:55:51 +1100 Subject: [PATCH] CustomData: validate layer indices, assert assigned values are valid Mesh.validate(..) now corrects invalid layer indices. Add assertion to prevent invalid values being set #105860. --- .../blender/blenkernel/intern/customdata.cc | 48 +++++++++++++++---- .../blenkernel/intern/mesh_validate.cc | 47 ++++++++++++++++-- 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index 0d7f55a6f26..feb8ed226eb 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -2583,8 +2583,12 @@ const char *CustomData_get_render_layer_name(const CustomData *data, const int t void CustomData_set_layer_active(CustomData *data, const int type, const int n) { +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { + BLI_assert(uint(n) < uint(layer_num)); data->layers[i].active = n; } } @@ -2592,8 +2596,12 @@ void CustomData_set_layer_active(CustomData *data, const int type, const int n) void CustomData_set_layer_render(CustomData *data, const int type, const int n) { +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { + BLI_assert(uint(n) < uint(layer_num)); data->layers[i].active_rnd = n; } } @@ -2601,8 +2609,12 @@ void CustomData_set_layer_render(CustomData *data, const int type, const int n) void CustomData_set_layer_clone(CustomData *data, const int type, const int n) { +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { + BLI_assert(uint(n) < uint(layer_num)); data->layers[i].active_clone = n; } } @@ -2610,8 +2622,12 @@ void CustomData_set_layer_clone(CustomData *data, const int type, const int n) void CustomData_set_layer_stencil(CustomData *data, const int type, const int n) { +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { + BLI_assert(uint(n) < uint(layer_num)); data->layers[i].active_mask = n; } } @@ -2619,48 +2635,64 @@ void CustomData_set_layer_stencil(CustomData *data, const int type, const int n) void CustomData_set_layer_active_index(CustomData *data, const int type, const int n) { - const int layer_index = data->typemap[type]; +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif + const int layer_index = n - data->typemap[type]; BLI_assert(customdata_typemap_is_valid(data)); for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { - data->layers[i].active = n - layer_index; + BLI_assert(uint(layer_index) < uint(layer_num)); + data->layers[i].active = layer_index; } } } void CustomData_set_layer_render_index(CustomData *data, const int type, const int n) { - const int layer_index = data->typemap[type]; +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif + const int layer_index = n - data->typemap[type]; BLI_assert(customdata_typemap_is_valid(data)); for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { - data->layers[i].active_rnd = n - layer_index; + BLI_assert(uint(layer_index) < uint(layer_num)); + data->layers[i].active_rnd = layer_index; } } } void CustomData_set_layer_clone_index(CustomData *data, const int type, const int n) { - const int layer_index = data->typemap[type]; +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif + const int layer_index = n - data->typemap[type]; BLI_assert(customdata_typemap_is_valid(data)); for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { - data->layers[i].active_clone = n - layer_index; + BLI_assert(uint(layer_index) < uint(layer_num)); + data->layers[i].active_clone = layer_index; } } } void CustomData_set_layer_stencil_index(CustomData *data, const int type, const int n) { - const int layer_index = data->typemap[type]; +#ifndef NDEBUG + const int layer_num = CustomData_number_of_layers(data, type); +#endif + const int layer_index = n - data->typemap[type]; BLI_assert(customdata_typemap_is_valid(data)); for (int i = 0; i < data->totlayer; i++) { if (data->layers[i].type == type) { - data->layers[i].active_mask = n - layer_index; + BLI_assert(uint(layer_index) < uint(layer_num)); + data->layers[i].active_mask = layer_index; } } } diff --git a/source/blender/blenkernel/intern/mesh_validate.cc b/source/blender/blenkernel/intern/mesh_validate.cc index f15020c9c46..34722586e74 100644 --- a/source/blender/blenkernel/intern/mesh_validate.cc +++ b/source/blender/blenkernel/intern/mesh_validate.cc @@ -929,16 +929,57 @@ static bool mesh_validate_customdata(CustomData *data, PRINT_MSG("%s: Checking %d CD layers...\n", __func__, data->totlayer); + /* Set dummy values so the layer-type is always initialized on first access. */ + int layer_num = -1; + int layer_num_type = -1; + while (i < data->totlayer) { CustomDataLayer *layer = &data->layers[i]; bool ok = true; + /* Count layers when the type changes. */ + if (layer_num_type != layer->type) { + layer_num = CustomData_number_of_layers(data, layer->type); + layer_num_type = layer->type; + } + + /* Validate active index, for a time this could be set to a negative value, see: #105860. */ + int *active_index_array[] = { + &layer->active, + &layer->active_rnd, + &layer->active_clone, + &layer->active_mask, + }; + for (int *active_index : Span(active_index_array, ARRAY_SIZE(active_index_array))) { + if (*active_index < 0) { + PRINT_ERR("\tCustomDataLayer type %d has a negative active index (%d)\n", + layer->type, + *active_index); + if (do_fixes) { + *active_index = 0; + has_fixes = true; + } + } + else { + if (*active_index >= layer_num) { + PRINT_ERR("\tCustomDataLayer type %d has an out of bounds active index (%d >= %d)\n", + layer->type, + *active_index, + layer_num); + if (do_fixes) { + BLI_assert(layer_num > 0); + *active_index = layer_num - 1; + has_fixes = true; + } + } + } + } + if (CustomData_layertype_is_singleton(layer->type)) { - const int layer_tot = CustomData_number_of_layers(data, layer->type); - if (layer_tot > 1) { + if (layer_num > 1) { PRINT_ERR("\tCustomDataLayer type %d is a singleton, found %d in Mesh structure\n", layer->type, - layer_tot); + layer_num); ok = false; } }