blender
/
blender
129
398
Fork 572
Code Issues 6.2k Pull Requests 750 Projects 19 Wiki Activity

Attributes: Validate some builtin attributes for untrusted inputs 

We expect some builtin attributes to have positive values or values
within a certain range, but currently there some cases where users
can set attributes to arbitrary values: the store named attribute node,
and the output attributes of the geometry nodes modifier. The set
material index node also needs validation.

This patch adds an `AttributeValidator` to the attribute API, which
can be used to correct values from these untrusted inputs if necessary.
As an alternative to D15548, this approach makes it much easier to
understand when validation is being applied, without the need to add
arguments to every attribute API method or complicate the virtual
array system.

Currently validation is provided with a multi-function. That integrates
well with the field evaluations that set these values now, but it could
be wrapped to be friendlier to other areas of Blender in the future.

The Python API is not handled here either. Currently I would prefer to
wait until we can integrate the C++ and C attribute APIs better before
addressing that.

Fixes T100952

Differential Revision: https://developer.blender.org/D15990
This commit is contained in:
Hans Goudey 2022-09-17 14:38:30 -05:00
parent 6069cab442
commit 8934f00ac5
Notes: blender-bot 2023-02-14 09:02:41 +01:00 
Referenced by commit 4f284873d0, Fix: Crash after recent attributes commit
Referenced by issue #100749, Blender LTS: Maintenance Task 3.3
Referenced by issue #102926, Crash: Geometry nodes, Set spline resolution to negative will crash Blender
Referenced by issue #100952, Geometry nodes lets users create negative material indices
9 changed files with 151 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
source/blender/blenkernel/BKE_attribute.hh
View File

@ -16,6 +16,10 @@
struct Mesh;
struct PointCloud;
namespace blender::fn {
class MultiFunction;
class GField;
} // namespace blender::fn
namespace blender::bke {
@ -162,6 +166,27 @@ template<typename T> struct AttributeReader {
}
};
/**
* A utility to make sure attribute values are valid, for attributes like "material_index" which
* can only be positive, or attributes that represent enum options. This is usually only necessary
* when writing attributes from an untrusted/arbitrary user input.
*/
struct AttributeValidator {
/**
* Single input, single output function that corrects attribute values if necessary.
*/
const fn::MultiFunction *function;
operator bool() const
{
return this->function != nullptr;
}
/**
* Return a field that creates corrected attribute values.
*/
fn::GField validate_field_if_necessary(const fn::GField &field) const;
};
/**
* Result when looking up an attribute from some geometry with read and write access. After writing
* to the attribute, the #finish method has to be called. This may invalidate caches based on this
@ -343,7 +368,7 @@ struct AttributeAccessorFunctions {
eAttrDomain to_domain);
bool (*for_all)(const void *owner,
FunctionRef<bool(const AttributeIDRef &, const AttributeMetaData &)> fn);
AttributeValidator (*lookup_validator)(const void *owner, const AttributeIDRef &attribute_id);
GAttributeWriter (*lookup_for_write)(void *owner, const AttributeIDRef &attribute_id);
bool (*remove)(void *owner, const AttributeIDRef &attribute_id);
bool (*add)(void *owner,
@ -497,6 +522,14 @@ class AttributeAccessor {
return VArray<T>::ForSingle(default_value, this->domain_size(domain));
}
/**
* Same as the generic version above, but should be used when the type is known at compile time.
*/
AttributeValidator lookup_validator(const AttributeIDRef &attribute_id) const
{
return fn_->lookup_validator(owner_, attribute_id);
}
/**
* Interpolate data from one domain to another.
*/

2
source/blender/blenkernel/BKE_curves.hh
View File

@ -218,7 +218,7 @@ class CurvesGeometry : public ::CurvesGeometry {
/**
* How many evaluated points to create for each segment when evaluating Bezier,
* Catmull Rom, and NURBS curves. On the curve domain.
* Catmull Rom, and NURBS curves. On the curve domain. Values must be zero or greater.
*/
VArray<int> resolution() const;
/** Mutable access to curve resolution. Call #tag_topology_changed after changes. */

11
source/blender/blenkernel/intern/attribute_access.cc
View File

@ -19,6 +19,8 @@
#include "BLI_math_vec_types.hh"
#include "BLI_span.hh"
#include "FN_field.hh"
#include "BLT_translation.h"
#include "CLG_log.h"
@ -945,6 +947,15 @@ GSpanAttributeWriter MutableAttributeAccessor::lookup_or_add_for_write_only_span
return {};
}
fn::GField AttributeValidator::validate_field_if_necessary(const fn::GField &field) const
{
if (function) {
auto validate_op = fn::FieldOperation::Create(*function, {field});
return fn::GField(validate_op);
}
return field;
}
Vector<AttributeTransferData> retrieve_attributes_for_transfer(
const bke::AttributeAccessor src_attributes,
bke::MutableAttributeAccessor dst_attributes,

40
source/blender/blenkernel/intern/attribute_access_intern.hh
View File

@ -53,6 +53,7 @@ class BuiltinAttributeProvider {
const CreatableEnum createable_;
const WritableEnum writable_;
const DeletableEnum deletable_;
const AttributeValidator validator_;
public:
BuiltinAttributeProvider(std::string name,
@ -60,13 +61,15 @@ class BuiltinAttributeProvider {
const eCustomDataType data_type,
const CreatableEnum createable,
const WritableEnum writable,
const DeletableEnum deletable)
const DeletableEnum deletable,
AttributeValidator validator = {})
: name_(std::move(name)),
domain_(domain),
data_type_(data_type),
createable_(createable),
writable_(writable),
deletable_(deletable)
deletable_(deletable),
validator_(validator)
{
}
@ -90,6 +93,11 @@ class BuiltinAttributeProvider {
{
return data_type_;
}
AttributeValidator validator() const
{
return validator_;
}
};
/**
@ -241,9 +249,15 @@ class BuiltinCustomDataLayerProvider final : public BuiltinAttributeProvider {
const CustomDataAccessInfo custom_data_access,
const AsReadAttribute as_read_attribute,
const AsWriteAttribute as_write_attribute,
const UpdateOnChange update_on_write)
: BuiltinAttributeProvider(
std::move(attribute_name), domain, attribute_type, creatable, writable, deletable),
const UpdateOnChange update_on_write,
const AttributeValidator validator = {})
: BuiltinAttributeProvider(std::move(attribute_name),
domain,
attribute_type,
creatable,
writable,
deletable,
validator),
stored_type_(stored_type),
custom_data_access_(custom_data_access),
as_read_attribute_(as_read_attribute),
@ -378,6 +392,21 @@ inline bool for_all(const void *owner,
return true;
}
template<const ComponentAttributeProviders &providers>
inline AttributeValidator lookup_validator(const void * /*owner*/,
const blender::bke::AttributeIDRef &attribute_id)
{
if (!attribute_id.is_named()) {
return {};
}
const auto &builtin_providers = providers.builtin_attribute_providers();
const BuiltinAttributeProvider *provider = builtin_providers.lookup_as(attribute_id.name());
if (!provider) {
return {};
}
return provider->validator();
}
template<const ComponentAttributeProviders &providers>
inline bool contains(const void *owner, const blender::bke::AttributeIDRef &attribute_id)
{
@ -489,6 +518,7 @@ inline AttributeAccessorFunctions accessor_functions_for_providers()
lookup<providers>,
nullptr,
for_all<providers>,
lookup_validator<providers>,
lookup_for_write<providers>,
remove<providers>,
add<providers>};

55
source/blender/blenkernel/intern/geometry_component_curves.cc
View File

@ -12,6 +12,8 @@
#include "BKE_geometry_set.hh"
#include "BKE_lib_id.h"
#include "FN_multi_function_builder.hh"
#include "attribute_access_intern.hh"
using blender::GVArray;
@ -426,6 +428,12 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
make_array_write_attribute<float3>,
tag_component_positions_changed);
static const fn::CustomMF_SI_SO<int8_t, int8_t> handle_type_clamp{
"Handle Type Validate",
[](int8_t value) {
return std::clamp<int8_t>(value, BEZIER_HANDLE_FREE, BEZIER_HANDLE_ALIGN);
},
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider handle_type_right("handle_type_right",
ATTR_DOMAIN_POINT,
CD_PROP_INT8,
@ -436,7 +444,8 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
point_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_topology_changed);
tag_component_topology_changed,
AttributeValidator{&handle_type_clamp});
static BuiltinCustomDataLayerProvider handle_type_left("handle_type_left",
ATTR_DOMAIN_POINT,
@ -448,7 +457,8 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
point_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_topology_changed);
tag_component_topology_changed,
AttributeValidator{&handle_type_clamp});
static BuiltinCustomDataLayerProvider nurbs_weight("nurbs_weight",
ATTR_DOMAIN_POINT,
@ -462,6 +472,10 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
make_array_write_attribute<float>,
tag_component_positions_changed);
static const fn::CustomMF_SI_SO<int8_t, int8_t> nurbs_order_clamp{
"NURBS Order Validate",
[](int8_t value) { return std::max<int8_t>(value, 0); },
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider nurbs_order("nurbs_order",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
@ -472,8 +486,15 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
curve_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_topology_changed);
tag_component_topology_changed,
AttributeValidator{&nurbs_order_clamp});
static const fn::CustomMF_SI_SO<int8_t, int8_t> normal_mode_clamp{
"Normal Mode Validate",
[](int8_t value) {
return std::clamp<int8_t>(value, NORMAL_MODE_MINIMUM_TWIST, NORMAL_MODE_Z_UP);
},
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider normal_mode("normal_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
@ -484,8 +505,15 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
curve_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_normals_changed);
tag_component_normals_changed,
AttributeValidator{&normal_mode_clamp});
static const fn::CustomMF_SI_SO<int8_t, int8_t> knots_mode_clamp{
"Knots Mode Validate",
[](int8_t value) {
return std::clamp<int8_t>(value, NURBS_KNOT_MODE_NORMAL, NURBS_KNOT_MODE_ENDPOINT_BEZIER);
},
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider nurbs_knots_mode("knots_mode",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
@ -496,8 +524,15 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
curve_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_topology_changed);
tag_component_topology_changed,
AttributeValidator{&knots_mode_clamp});
static const fn::CustomMF_SI_SO<int8_t, int8_t> curve_type_clamp{
"Curve Type Validate",
[](int8_t value) {
return std::clamp<int8_t>(value, CURVE_TYPE_CATMULL_ROM, CURVE_TYPES_NUM);
},
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider curve_type("curve_type",
ATTR_DOMAIN_CURVE,
CD_PROP_INT8,
@ -508,8 +543,13 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
curve_access,
make_array_read_attribute<int8_t>,
make_array_write_attribute<int8_t>,
tag_component_curve_types_changed);
tag_component_curve_types_changed,
AttributeValidator{&curve_type_clamp});
static const fn::CustomMF_SI_SO<int, int> resolution_clamp{
"Resolution Validate",
[](int value) { return std::max<int>(value, 0); },
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider resolution("resolution",
ATTR_DOMAIN_CURVE,
CD_PROP_INT32,
@ -520,7 +560,8 @@ static ComponentAttributeProviders create_attribute_providers_for_curve()
curve_access,
make_array_read_attribute<int>,
make_array_write_attribute<int>,
tag_component_topology_changed);
tag_component_topology_changed,
AttributeValidator{&resolution_clamp});
static BuiltinCustomDataLayerProvider cyclic("cyclic",
ATTR_DOMAIN_CURVE,

12
source/blender/blenkernel/intern/geometry_component_mesh.cc
View File

@ -14,6 +14,8 @@
#include "BKE_lib_id.h"
#include "BKE_mesh.h"
#include "FN_multi_function_builder.hh"
#include "attribute_access_intern.hh"
extern "C" MDeformVert *BKE_object_defgroup_data_create(ID *id);
@ -1217,6 +1219,13 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
make_array_write_attribute<int>,
nullptr);
static const fn::CustomMF_SI_SO<int, int> material_index_clamp{
"Material Index Validate",
[](int value) {
/* Use #short for the maximum since many areas still use that type for indices. */
return std::clamp<int>(value, 0, std::numeric_limits<short>::max());
},
fn::CustomMF_presets::AllSpanOrSingle()};
static BuiltinCustomDataLayerProvider material_index("material_index",
ATTR_DOMAIN_FACE,
CD_PROP_INT32,
@ -1227,7 +1236,8 @@ static ComponentAttributeProviders create_attribute_providers_for_mesh()
face_access,
make_array_read_attribute<int>,
make_array_write_attribute<int>,
nullptr);
nullptr,
AttributeValidator{&material_index_clamp});
static BuiltinCustomDataLayerProvider shade_smooth(
"shade_smooth",

6
source/blender/modifiers/intern/MOD_nodes.cc
View File

@ -114,7 +114,9 @@ using blender::StringRef;
using blender::StringRefNull;
using blender::Vector;
using blender::bke::AttributeMetaData;
using blender::bke::AttributeValidator;
using blender::fn::Field;
using blender::fn::FieldOperation;
using blender::fn::GField;
using blender::fn::ValueOrField;
using blender::fn::ValueOrFieldCPPType;
@ -1046,13 +1048,15 @@ static Vector<OutputAttributeToStore> compute_attributes_to_store(
blender::fn::FieldEvaluator field_evaluator{field_context, domain_size};
for (const OutputAttributeInfo &output_info : outputs_info) {
const CPPType &type = output_info.field.cpp_type();
const AttributeValidator validator = attributes.lookup_validator(output_info.name);
OutputAttributeToStore store{
component_type,
domain,
output_info.name,
GMutableSpan{
type, MEM_malloc_arrayN(domain_size, type.size(), __func__), domain_size}};
field_evaluator.add_with_destination(output_info.field, store.data);
GField field = validator.validate_field_if_necessary(output_info.field);
field_evaluator.add_with_destination(std::move(field), store.data);
attributes_to_store.append(store);
}
field_evaluator.evaluate();

4
source/blender/nodes/geometry/nodes/node_geo_set_material_index.cc
View File

@ -24,11 +24,13 @@ static void set_material_index_in_component(GeometryComponent &component,
MutableAttributeAccessor attributes = *component.attributes_for_write();
bke::GeometryFieldContext field_context{component, domain};
const bke::AttributeValidator validator = attributes.lookup_validator("material_index");
AttributeWriter<int> indices = attributes.lookup_or_add_for_write<int>("material_index", domain);
fn::FieldEvaluator evaluator{field_context, domain_size};
evaluator.set_selection(selection_field);
evaluator.add_with_destination(index_field, indices.varray);
evaluator.add_with_destination(validator.validate_field_if_necessary(index_field),
indices.varray);
evaluator.evaluate();
indices.finish();
}

4
source/blender/nodes/geometry/nodes/node_geo_store_named_attribute.cc
View File

@ -103,6 +103,7 @@ static void try_capture_field_on_geometry(GeometryComponent &component,
const CPPType &type = field.cpp_type();
const eCustomDataType data_type = bke::cpp_type_to_custom_data_type(type);
const bke::AttributeValidator validator = attributes.lookup_validator(name);
/* Could avoid allocating a new buffer if:
* - We are writing to an attribute that exists already with the correct domain and type.
@ -110,7 +111,8 @@ static void try_capture_field_on_geometry(GeometryComponent &component,
void *buffer = MEM_mallocN(type.size() * domain_size, __func__);
fn::FieldEvaluator evaluator{field_context, &mask};
evaluator.add_with_destination(field, GMutableSpan{type, buffer, domain_size});
evaluator.add_with_destination(validator.validate_field_if_necessary(field),
GMutableSpan{type, buffer, domain_size});
evaluator.evaluate();
if (GAttributeWriter attribute = attributes.lookup_for_write(name)) {