CVE-2022-32746 ldb: Ensure shallow copy modifications do not affect original message

Using the newly added ldb flag, we can now detect when a message has
been shallow-copied so that its elements share their values with the
original message elements. Then when adding values to the copied
message, we now make a copy of the shared values array first.

This should prevent a use-after-free that occurred in LDB modules when
new values were added to a shallow copy of a message by calling
talloc_realloc() on the original values array, invalidating the 'values'
pointer in the original message element. The original values pointer can
later be used in the database audit logging module which logs database
requests, and potentially cause a crash.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=15009

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>

diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
index 2a9ce384bb98f3aa0c8d03e20f796a96cff4ce02..44d3b29e9a72b866590e2850183d320839cd1f47 100644 (file)
--- a/lib/ldb/common/ldb_msg.c
+++ b/lib/ldb/common/ldb_msg.c
@@ -417,6 +417,47 @@ int ldb_msg_add(struct ldb_message *msg,
        return LDB_SUCCESS;
 }
 
+/*
+ * add a value to a message element
+ */
+int ldb_msg_element_add_value(TALLOC_CTX *mem_ctx,
+                             struct ldb_message_element *el,
+                             const struct ldb_val *val)
+{
+       struct ldb_val *vals;
+
+       if (el->flags & LDB_FLAG_INTERNAL_SHARED_VALUES) {
+               /*
+                * Another message is using this message element's values array,
+                * so we don't want to make any modifications to the original
+                * message, or potentially invalidate its own values by calling
+                * talloc_realloc(). Make a copy instead.
+                */
+               el->flags &= ~LDB_FLAG_INTERNAL_SHARED_VALUES;
+
+               vals = talloc_array(mem_ctx, struct ldb_val,
+                                   el->num_values + 1);
+               if (vals == NULL) {
+                       return LDB_ERR_OPERATIONS_ERROR;
+               }
+
+               if (el->values != NULL) {
+                       memcpy(vals, el->values, el->num_values * sizeof(struct ldb_val));
+               }
+       } else {
+               vals = talloc_realloc(mem_ctx, el->values, struct ldb_val,
+                                     el->num_values + 1);
+               if (vals == NULL) {
+                       return LDB_ERR_OPERATIONS_ERROR;
+               }
+       }
+       el->values = vals;
+       el->values[el->num_values] = *val;
+       el->num_values++;
+
+       return LDB_SUCCESS;
+}
+
 /*
   add a value to a message
 */
@@ -426,7 +467,6 @@ int ldb_msg_add_value(struct ldb_message *msg,
                      struct ldb_message_element **return_el)
 {
        struct ldb_message_element *el;
-       struct ldb_val *vals;
        int ret;
 
        el = ldb_msg_find_element(msg, attr_name);
@@ -437,14 +477,10 @@ int ldb_msg_add_value(struct ldb_message *msg,
                }
        }
 
-       vals = talloc_realloc(msg->elements, el->values, struct ldb_val,
-                             el->num_values+1);
-       if (!vals) {
-               return LDB_ERR_OPERATIONS_ERROR;
+       ret = ldb_msg_element_add_value(msg->elements, el, val);
+       if (ret != LDB_SUCCESS) {
+               return ret;
        }
-       el->values = vals;
-       el->values[el->num_values] = *val;
-       el->num_values++;
 
        if (return_el) {
                *return_el = el;

diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
index bc44157eaf472098cfded28efd22cada6d147559..129beefeaf56fde4f7fbc8eed673f6a4a05d00af 100644 (file)
--- a/lib/ldb/include/ldb.h
+++ b/lib/ldb/include/ldb.h
@@ -1981,6 +1981,12 @@ int ldb_msg_add_empty(struct ldb_message *msg,
                int flags,
                struct ldb_message_element **return_el);
 
+/**
+   add a value to a message element
+*/
+int ldb_msg_element_add_value(TALLOC_CTX *mem_ctx,
+                             struct ldb_message_element *el,
+                             const struct ldb_val *val);
 /**
    add a element to a ldb_message
 */

diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 5ce4c0a5e3323cdb0f43b69783c2308f4af46ab5..577b2a3387383bf7fe11bb932cec35b2925a3fd9 100644 (file)
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -816,7 +816,7 @@ int samdb_msg_add_addval(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx,
                         const char *value)
 {
        struct ldb_message_element *el;
-       struct ldb_val val, *vals;
+       struct ldb_val val;
        char *v;
        unsigned int i;
        bool found = false;
@@ -851,14 +851,10 @@ int samdb_msg_add_addval(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx,
                }
        }
 
-       vals = talloc_realloc(msg->elements, el->values, struct ldb_val,
-                             el->num_values + 1);
-       if (vals == NULL) {
+       ret = ldb_msg_element_add_value(msg->elements, el, &val);
+       if (ret != LDB_SUCCESS) {
                return ldb_oom(sam_ldb);
        }
-       el->values = vals;
-       el->values[el->num_values] = val;
-       ++(el->num_values);
 
        return LDB_SUCCESS;
 }
@@ -872,7 +868,7 @@ int samdb_msg_add_delval(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx,
                         const char *value)
 {
        struct ldb_message_element *el;
-       struct ldb_val val, *vals;
+       struct ldb_val val;
        char *v;
        unsigned int i;
        bool found = false;
@@ -907,14 +903,10 @@ int samdb_msg_add_delval(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx,
                }
        }
 
-       vals = talloc_realloc(msg->elements, el->values, struct ldb_val,
-                             el->num_values + 1);
-       if (vals == NULL) {
+       ret = ldb_msg_element_add_value(msg->elements, el, &val);
+       if (ret != LDB_SUCCESS) {
                return ldb_oom(sam_ldb);
        }
-       el->values = vals;
-       el->values[el->num_values] = val;
-       ++(el->num_values);
 
        return LDB_SUCCESS;
 }