[Bf-blender-cvs] [9c6c4cc5308] temp-T97352-3d-texturing-seam-bleeding-b2: Attributes: Validate some builtin attributes for untrusted inputs
Hans Goudey
noreply at git.blender.org
Tue Sep 20 10:32:12 CEST 2022
Commit: 9c6c4cc5308e663043acc2d20a8fc6ee361ac875
Author: Hans Goudey
Date: Sat Sep 17 14:38:30 2022 -0500
Branches: temp-T97352-3d-texturing-seam-bleeding-b2
https://developer.blender.org/rB9c6c4cc5308e663043acc2d20a8fc6ee361ac875
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
===================================================================
M source/blender/blenkernel/BKE_attribute.hh
M source/blender/blenkernel/BKE_curves.hh
M source/blender/blenkernel/intern/attribute_access.cc
M source/blender/blenkernel/intern/attribute_access_intern.hh
M source/blender/blenkernel/intern/geometry_component_curves.cc
M source/blender/blenkernel/intern/geometry_component_mesh.cc
M source/blender/modifiers/intern/MOD_nodes.cc
M source/blender/nodes/geometry/nodes/node_geo_set_material_index.cc
M source/blender/nodes/geometry/nodes/node_geo_store_named_attribute.cc
===================================================================
diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh
index fbdacee139c..946a7d21580 100644
--- a/source/blender/blenkernel/BKE_attribute.hh
+++ b/source/blender/blenkernel/BKE_attribute.hh
@@ -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.
*/
diff --git a/source/blender/blenkernel/BKE_curves.hh b/source/blender/blenkernel/BKE_curves.hh
index 9f150c13d6e..6758e5c268b 100644
--- a/source/blender/blenkernel/BKE_curves.hh
+++ b/source/blender/blenkernel/BKE_curves.hh
@@ -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. */
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index e7c1e35f851..df7787986db 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -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,
diff --git a/source/blender/blenkernel/intern/attribute_access_intern.hh b/source/blender/blenkernel/intern/attribute_access_intern.hh
index 8050f45da94..e9023e28297 100644
--- a/source/blender/blenkernel/intern/attribute_access_intern.hh
+++ b/source/blender/blenkernel/intern/attribute_access_intern.hh
@@ -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>};
diff --git a/source/blender/blenkernel/intern/geometry_component_curves.cc b/source/blender/blenkernel/intern/geometry_component_curves.cc
index 83a35534c01..d8cfe1374dc 100644
--- a/source/blender/blenkernel/intern/geometry_component_curves.cc
+++ b/source/blender/blenkernel/intern/geometry_component_curves.cc
@@ -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>,
-
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list