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>

3 files changed, 56 insertions, 22 deletions

diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
index 2a9ce384bb9..44d3b29e9a7 100644
--- a/lib/ldb/common/ldb_msg.c
+++ b/lib/ldb/common/ldb_msg.c
@@ -418,6 +418,47 @@ int ldb_msg_add(struct ldb_message *msg,
 }
 
 /*
+ * 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
 */
 int ldb_msg_add_value(struct ldb_message *msg,
@@ -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 bc44157eaf4..129beefeaf5 100644
--- a/lib/ldb/include/ldb.h
+++ b/lib/ldb/include/ldb.h
@@ -1982,6 +1982,12 @@ int ldb_msg_add_empty(struct ldb_message *msg,
 		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
 */
 int ldb_msg_add(struct ldb_message *msg,
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 5ce4c0a5e33..577b2a33873 100644
--- 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;
 }