More improvements around allocators

* Remove public constructors from owned_object, as these just duplicate
  ddwaf_object::make_* factory methods and may have confusing affects
  around some implicit conversions.
* make borrowed objects destroy the current value when they're assigned
  to from an owned_object&&. This prevents some usages of
  to_borrowed(x), where x is uninitialized memory.
* remove more implicit usages of the default allocator, causing runtime
  errors.

Author: cataphract

fuzzer/http_endpoint_fingerprint/src/main.cpp

@@ -17,15 +17,15 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
 {
     random_buffer buffer{bytes, size};
 
-    auto query = owned_object::make_map();
+    auto query = owned_object::make_map(0, ddwaf::memory::get_default_resource());
     auto query_size = buffer.get<uint8_t>();
     for (uint8_t i = 0; i < query_size; ++i) {
         auto key = buffer.get<std::string_view>();
         auto value = buffer.get<std::string_view>();
         query.emplace(key, value);
     }
 
-    auto body = owned_object::make_map();
+    auto body = owned_object::make_map(0, ddwaf::memory::get_default_resource());
     auto body_size = buffer.get<uint8_t>();
     for (uint8_t i = 0; i < body_size; ++i) {
         auto key = buffer.get<std::string_view>();
@@ -41,8 +41,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
         gen.eval_impl({.address = {}, .key_path = {}, .value = buffer.get<std::string_view>()},
             {.address = {}, .key_path = {}, .value = buffer.get<std::string_view>()},
             {{.address = {}, .key_path = {}, .value = query}},
-            {{.address = {}, .key_path = {}, .value = body}}, cache, memory::get_default_resource(),
-            deadline);
+            {{.address = {}, .key_path = {}, .value = body}}, cache,
+            ddwaf::memory::get_default_resource(), deadline);
 
     return 0;
 }

fuzzer/http_header_fingerprint/src/main.cpp

@@ -22,7 +22,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
 
     random_buffer buffer{bytes, size};
 
-    auto header = owned_object::make_map();
+    auto header = owned_object::make_map(0, ddwaf::memory::get_default_resource());
     auto header_size = buffer.get<uint8_t>();
     for (uint8_t i = 0; i < header_size; ++i) {
         auto value = buffer.get<std::string_view>();
@@ -41,7 +41,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
     processor_cache cache;
     ddwaf::timer deadline{2s};
     auto output = gen.eval_impl({.address = {}, .key_path = {}, .value = header}, cache,
-        memory::get_default_resource(), deadline);
+        ddwaf::memory::get_default_resource(), deadline);
 
     return 0;
 }

fuzzer/http_network_fingerprint/src/main.cpp

@@ -21,7 +21,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
 
     random_buffer buffer{bytes, size};
 
-    auto header = owned_object::make_map();
+    auto header = owned_object::make_map(0, ddwaf::memory::get_default_resource());
     auto header_size = buffer.get<uint8_t>();
     for (uint8_t i = 0; i < header_size; ++i) {
         auto value = buffer.get<std::string_view>();
@@ -40,7 +40,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
     processor_cache cache;
     ddwaf::timer deadline{2s};
     auto output = gen.eval_impl({.address = {}, .key_path = {}, .value = header}, cache,
-        memory::get_default_resource(), deadline);
+        ddwaf::memory::get_default_resource(), deadline);
 
     return 0;
 }

fuzzer/jwt_decode/src/main.cpp

@@ -18,15 +18,16 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
     // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
     std::string_view value{reinterpret_cast<const char *>(bytes), size};
 
-    auto headers = object_builder::map({{"authorization", value}});
+    auto headers =
+        object_builder::map({{"authorization", value}}, ddwaf::memory::get_default_resource());
 
     jwt_decode gen{"id", {}, {}, false, true};
 
     processor_cache cache;
     ddwaf::timer deadline{2s};
     static const std::vector<std::variant<std::string, int64_t>> key_path{"authorization"};
     auto output = gen.eval_impl({.address = {}, .key_path = key_path, .value = headers}, cache,
-        memory::get_default_resource(), deadline);
+        ddwaf::memory::get_default_resource(), deadline);
 
     return 0;
 }

fuzzer/session_fingerprint/src/main.cpp

@@ -17,7 +17,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
 {
     random_buffer buffer{bytes, size};
 
-    auto cookies = owned_object::make_map();
+    auto cookies = owned_object::make_map(0, ddwaf::memory::get_default_resource());
     auto cookies_size = buffer.get<uint8_t>();
     for (uint8_t i = 0; i < cookies_size; ++i) {
         auto key = buffer.get<std::string_view>();
@@ -34,7 +34,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *bytes, size_t size)
     auto output = gen.eval_impl({{.address = {}, .key_path = {}, .value = cookies}},
         {{.address = {}, .key_path = {}, .value = buffer.get<std::string_view>()}},
         {{.address = {}, .key_path = {}, .value = buffer.get<std::string_view>()}}, cache,
-        memory::get_default_resource(), deadline);
+        ddwaf::memory::get_default_resource(), deadline);
 
     return 0;
 }

include/ddwaf.h

@@ -219,10 +219,9 @@ typedef void (*ddwaf_log_cb)(
  *
  * @return Handle to the WAF instance or NULL on error.
  *
- * @note If config is NULL, default values will be used, including the default
- *       free function (ddwaf_object_free).
- *
  * @note If ruleset is NULL, the diagnostics object will not be initialised.
+ *
+ * @note The deallocation of the diagnostics must be made with default allocator.
  **/
 ddwaf_handle ddwaf_init(const ddwaf_object *ruleset,  ddwaf_object *diagnostics);
 
@@ -279,6 +278,7 @@ const char *const *ddwaf_known_actions(const ddwaf_handle handle, uint32_t *size
  * Context object to perform matching using the provided WAF instance.
  *
  * @param handle Handle of the WAF instance containing the ruleset definition. (nonnull)
+ * @param output Allocator used to serve output objects created during evaluation (nonnull)
 
  * @return Handle to the context instance.
  *
@@ -304,6 +304,9 @@ ddwaf_context ddwaf_context_init(const ddwaf_handle handle, ddwaf_allocator outp
  *    relevant address associated to the value, which can be of an arbitrary
  *    type.
  *
+ * @param alloc (nullable) Allocator used to free the data provided. If NULL,
+ *              the data will not be freed.
+ *
  * @param result (nullable) Object map containing the following items:
  *               - events: an array of the generated events.
  *               - actions: a map of the generated actions in the format:
@@ -318,6 +321,9 @@ ddwaf_context ddwaf_context_init(const ddwaf_handle handle, ddwaf_allocator outp
  *               This structure must be freed by the caller and will contain all
  *               specified keys when the value returned by ddwaf_context_eval is either
  *               DDWAF_OK or DDWAF_MATCH and will be empty otherwise.
+ *               IMPORTANT: This object is not allocated with the allocator
+ *               passed in this call. It uses the allocator given to
+ *               ddwaf_context_init instead.
  * @param timeout Maximum time budget in microseconds.
  *
  * @return Return code of the operation.
@@ -380,6 +386,9 @@ ddwaf_subcontext ddwaf_subcontext_init(ddwaf_context context);
  *    {string, <value>} in which each key represents the  relevant address
  *    associated to the value, which can be of an arbitrary type.
  *
+ * @param alloc (nullable) Allocator used to free the data provided. If NULL,
+ *              the data will not be freed.
+ *
  * @param result (nullable) Object map containing the following items:
  *               - events: an array of the generated events.
  *               - actions: a map of the generated actions in the format:
@@ -394,6 +403,9 @@ ddwaf_subcontext ddwaf_subcontext_init(ddwaf_context context);
  *               This structure must be freed by the caller and will contain all
  *               specified keys when the value returned by ddwaf_subcontext_eval is either
  *               DDWAF_OK or DDWAF_MATCH and will be empty otherwise.
+ *               IMPORTANT: This object is not allocated with the allocator
+ *               passed in this call. It uses the allocator given to
+ *               ddwaf_context_init instead.
  * @param timeout Maximum time budget in microseconds.
  *
  * @return Return code of the operation.
@@ -509,7 +521,7 @@ ddwaf_handle ddwaf_builder_build_instance(ddwaf_builder builder);
  *         of those matching the filter.
  *
  * @note This function is not thread-safe and the memory of the paths object must
- *       be freed by the caller.
+ *       be freed by the caller using the default allocator.
  **/
 uint32_t ddwaf_builder_get_config_paths(ddwaf_builder builder, ddwaf_object *paths, const char *filter, uint32_t filter_len);
 

src/attribute_collector.hpp

@@ -48,20 +48,40 @@ class attribute_collector {
     attribute_collector &operator=(attribute_collector &&other) noexcept = default;
     ~attribute_collector() = default;
 
-    // For types where owned_object has a single-argument constructor
+    bool insert(std::string_view key, bool value)
+    {
+        return insert(key, owned_object::make_boolean(value, attributes_.alloc()));
+    }
+
     template <typename T>
-    bool insert(std::string_view key, T &&value)
-        requires std::constructible_from<owned_object, T>
+    bool insert(std::string_view key, T value)
+        requires std::is_integral_v<T> && std::is_signed_v<T>
     {
-        return insert(key, owned_object{std::forward<T>(value)});
+        return insert(key, owned_object::make_signed(value, attributes_.alloc()));
     }
 
-    // For types that require an allocator
     template <typename T>
-    bool insert(std::string_view key, T &&value, nonnull_ptr<memory::memory_resource> alloc)
-        requires std::constructible_from<owned_object, T, nonnull_ptr<memory::memory_resource>>
+    bool insert(std::string_view key, T value)
+        requires std::is_integral_v<T> && std::is_unsigned_v<T> && (!std::same_as<T, bool>)
+    {
+        return insert(key, owned_object::make_unsigned(value, attributes_.alloc()));
+    }
+
+    template <typename T>
+    bool insert(std::string_view key, T value)
+        requires std::is_floating_point_v<T>
+    {
+        return insert(key, owned_object::make_float(value, attributes_.alloc()));
+    }
+
+    template <typename T>
+    bool insert(std::string_view key, T &&value)
+        requires std::convertible_to<T, std::string_view> &&
+                 (!std::is_integral_v<std::remove_cvref_t<T>>) &&
+                 (!std::is_floating_point_v<std::remove_cvref_t<T>>)
     {
-        return insert(key, owned_object{std::forward<T>(value), alloc});
+        return insert(key, owned_object::make_string(
+                               std::string_view(std::forward<T>(value)), attributes_.alloc()));
     }
 
     bool insert(std::string_view key, owned_object &&object);

src/dynamic_string.cpp

@@ -13,14 +13,15 @@ namespace ddwaf {
 
 owned_object dynamic_string::to_object(nonnull_ptr<memory::memory_resource> alloc)
 {
-    owned_object object;
-    if (size_ == capacity_ && alloc->is_equal(*alloc_)) {
-        // safety: the allocators compare equal; by definition this means alloc
-        // (the deallocator for the owned_object) can deallocate memory from
-        // alloc_ (the allocator for this dynamic_string)
-        object = owned_object::unsafe_make_string_nocopy(buffer_, size_, alloc);
-    } else {
-        object = owned_object::make_string(buffer_, size_, alloc);
+    bool should_copy = !(size_ == capacity_ && alloc->is_equal(*alloc_));
+    owned_object object = should_copy ? owned_object::make_string(buffer_, size_, alloc)
+                                      : // safety: the allocators compare equal; by definition
+                                        // this means alloc (the deallocator for the owned_object)
+                                        // can deallocate memory from alloc_ (the allocator for
+                                        // this dynamic_string)
+                              owned_object::unsafe_make_string_nocopy(buffer_, size_, alloc);
+
+    if (should_copy) {
         alloc_->deallocate(buffer_, capacity_, alignof(char));
     }
     buffer_ = nullptr;

src/evaluation_engine.cpp

@@ -42,7 +42,7 @@ void set_context_event_address(object_store &store)
         return;
     }
 
-    store.insert(event_addr_idx, event_addr, owned_object{true});
+    store.insert(event_addr_idx, event_addr, owned_object::make_boolean(true));
 }
 
 } // namespace