CVE-2022-32746 ldb: Make use of functions for appending to an ldb_message

This aims to minimise usage of the error-prone pattern of searching for
a just-added message element in order to make modifications to it (and
potentially finding the wrong element).

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

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

15 files changed, 183 insertions, 229 deletions

diff --git a/lib/ldb/ldb_map/ldb_map.c b/lib/ldb/ldb_map/ldb_map.c
index b453dff80d2..c7b0c228631 100644
--- a/lib/ldb/ldb_map/ldb_map.c
+++ b/lib/ldb/ldb_map/ldb_map.c
@@ -946,10 +946,7 @@ struct ldb_request *map_build_fixup_req(struct map_context *ac,
 	if ( ! dn || ! ldb_dn_validate(msg->dn)) {
 		goto failed;
 	}
-	if (ldb_msg_add_empty(msg, IS_MAPPED, LDB_FLAG_MOD_REPLACE, NULL) != 0) {
-		goto failed;
-	}
-	if (ldb_msg_add_string(msg, IS_MAPPED, dn) != 0) {
+	if (ldb_msg_append_string(msg, IS_MAPPED, dn, LDB_FLAG_MOD_REPLACE) != 0) {
 		goto failed;
 	}
 
diff --git a/lib/ldb/ldb_map/ldb_map_inbound.c b/lib/ldb/ldb_map/ldb_map_inbound.c
index 324295737da..50b9427c26c 100644
--- a/lib/ldb/ldb_map/ldb_map_inbound.c
+++ b/lib/ldb/ldb_map/ldb_map_inbound.c
@@ -569,12 +569,9 @@ static int map_modify_do_local(struct map_context *ac)
 		/* No local record present, add it instead */
 		/* Add local 'IS_MAPPED' */
 		/* TODO: use GUIDs here instead */
-		if (ldb_msg_add_empty(ac->local_msg, IS_MAPPED,
-					LDB_FLAG_MOD_ADD, NULL) != 0) {
-			return LDB_ERR_OPERATIONS_ERROR;
-		}
-		ret = ldb_msg_add_linearized_dn(ac->local_msg, IS_MAPPED,
-						ac->remote_req->op.mod.message->dn);
+		ret = ldb_msg_append_linearized_dn(ac->local_msg, IS_MAPPED,
+						   ac->remote_req->op.mod.message->dn,
+						   LDB_FLAG_MOD_ADD);
 		if (ret != 0) {
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
diff --git a/lib/ldb/modules/rdn_name.c b/lib/ldb/modules/rdn_name.c
index 25cffe07591..3cb62bf567b 100644
--- a/lib/ldb/modules/rdn_name.c
+++ b/lib/ldb/modules/rdn_name.c
@@ -308,16 +308,10 @@ static int rdn_rename_callback(struct ldb_request *req, struct ldb_reply *ares)
 	}
 	rdn_val = ldb_val_dup(msg, rdn_val_p);
 
-	if (ldb_msg_add_empty(msg, rdn_name, LDB_FLAG_MOD_REPLACE, NULL) != 0) {
+	if (ldb_msg_append_value(msg, rdn_name, &rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		goto error;
 	}
-	if (ldb_msg_add_value(msg, rdn_name, &rdn_val, NULL) != 0) {
-		goto error;
-	}
-	if (ldb_msg_add_empty(msg, "name", LDB_FLAG_MOD_REPLACE, NULL) != 0) {
-		goto error;
-	}
-	if (ldb_msg_add_value(msg, "name", &rdn_val, NULL) != 0) {
+	if (ldb_msg_append_value(msg, "name", &rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		goto error;
 	}
 
@@ -466,11 +460,7 @@ static int rdn_name_modify(struct ldb_module *module, struct ldb_request *req)
 		if (ret != 0) {
 			return ldb_module_oom(module);
 		}
-		ret = ldb_msg_add_empty(msg, rdn_name, LDB_FLAG_MOD_ADD, NULL);
-		if (ret != 0) {
-			return ldb_module_oom(module);
-		}
-		ret = ldb_msg_add_value(msg, rdn_name, &rdn_val, NULL);
+		ret = ldb_msg_append_value(msg, rdn_name, &rdn_val, LDB_FLAG_MOD_ADD);
 		if (ret != 0) {
 			return ldb_module_oom(module);
 		}
@@ -479,11 +469,7 @@ static int rdn_name_modify(struct ldb_module *module, struct ldb_request *req)
 		if (ret != 0) {
 			return ldb_module_oom(module);
 		}
-		ret = ldb_msg_add_empty(msg, "name", LDB_FLAG_MOD_ADD, NULL);
-		if (ret != 0) {
-			return ldb_module_oom(module);
-		}
-		ret = ldb_msg_add_value(msg, "name", &rdn_val, NULL);
+		ret = ldb_msg_append_value(msg, "name", &rdn_val, LDB_FLAG_MOD_ADD);
 		if (ret != 0) {
 			return ldb_module_oom(module);
 		}
diff --git a/source3/passdb/pdb_samba_dsdb.c b/source3/passdb/pdb_samba_dsdb.c
index 93e8f5bebe6..b2063825c04 100644
--- a/source3/passdb/pdb_samba_dsdb.c
+++ b/source3/passdb/pdb_samba_dsdb.c
@@ -2855,18 +2855,10 @@ static bool pdb_samba_dsdb_set_trusteddom_pw(struct pdb_methods *m,
 	}
 
 	msg->num_elements = 0;
-	ret = ldb_msg_add_empty(msg, "trustAuthOutgoing",
-				LDB_FLAG_MOD_REPLACE, NULL);
+	ret = ldb_msg_append_value(msg, "trustAuthOutgoing",
+				   &new_val, LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
-		DEBUG(0, ("ldb_msg_add_empty() failed\n"));
-		TALLOC_FREE(tmp_ctx);
-		ldb_transaction_cancel(state->ldb);
-		return false;
-	}
-	ret = ldb_msg_add_value(msg, "trustAuthOutgoing",
-				&new_val, NULL);
-	if (ret != LDB_SUCCESS) {
-		DEBUG(0, ("ldb_msg_add_value() failed\n"));
+		DEBUG(0, ("ldb_msg_append_value() failed\n"));
 		TALLOC_FREE(tmp_ctx);
 		ldb_transaction_cancel(state->ldb);
 		return false;
diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
index bcb0d087faf..cb9a082ebf6 100644
--- a/source4/dns_server/dnsserver_common.c
+++ b/source4/dns_server/dnsserver_common.c
@@ -1092,15 +1092,9 @@ WERROR dns_common_replace(struct ldb_context *samdb,
 	}
 
 	if (was_tombstoned || become_tombstoned) {
-		ret = ldb_msg_add_empty(msg, "dNSTombstoned",
-					LDB_FLAG_MOD_REPLACE, NULL);
-		if (ret != LDB_SUCCESS) {
-			werr = DNS_ERR(SERVER_FAILURE);
-			goto exit;
-		}
-
-		ret = ldb_msg_add_fmt(msg, "dNSTombstoned", "%s",
-				      become_tombstoned ? "TRUE" : "FALSE");
+		ret = ldb_msg_append_fmt(msg, LDB_FLAG_MOD_REPLACE,
+					 "dNSTombstoned", "%s",
+					 become_tombstoned ? "TRUE" : "FALSE");
 		if (ret != LDB_SUCCESS) {
 			werr = DNS_ERR(SERVER_FAILURE);
 			goto exit;
diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c
index 577b2a33873..10d6ea8883b 100644
--- a/source4/dsdb/common/util.c
+++ b/source4/dsdb/common/util.c
@@ -924,6 +924,16 @@ int samdb_msg_add_int(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct l
 	return ldb_msg_add_string(msg, attr_name, s);
 }
 
+int samdb_msg_add_int_flags(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			    const char *attr_name, int v, int flags)
+{
+	const char *s = talloc_asprintf(mem_ctx, "%d", v);
+	if (s == NULL) {
+		return ldb_oom(sam_ldb);
+	}
+	return ldb_msg_add_string_flags(msg, attr_name, s, flags);
+}
+
 /*
  * Add an unsigned int element to a message
  *
@@ -942,6 +952,12 @@ int samdb_msg_add_uint(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct
 	return samdb_msg_add_int(sam_ldb, mem_ctx, msg, attr_name, (int)v);
 }
 
+int samdb_msg_add_uint_flags(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			     const char *attr_name, unsigned int v, int flags)
+{
+	return samdb_msg_add_int_flags(sam_ldb, mem_ctx, msg, attr_name, (int)v, flags);
+}
+
 /*
   add a (signed) int64_t element to a message
 */
@@ -974,6 +990,68 @@ int samdb_msg_add_uint64(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struc
 }
 
 /*
+  append a int element to a message
+*/
+int samdb_msg_append_int(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+		      const char *attr_name, int v, int flags)
+{
+	const char *s = talloc_asprintf(mem_ctx, "%d", v);
+	if (s == NULL) {
+		return ldb_oom(sam_ldb);
+	}
+	return ldb_msg_append_string(msg, attr_name, s, flags);
+}
+
+/*
+ * Append an unsigned int element to a message
+ *
+ * The issue here is that we have not yet first cast to int32_t explicitly,
+ * before we cast to an signed int to printf() into the %d or cast to a
+ * int64_t before we then cast to a long long to printf into a %lld.
+ *
+ * There are *no* unsigned integers in Active Directory LDAP, even the RID
+ * allocations and ms-DS-Secondary-KrbTgt-Number are *signed* quantities.
+ * (See the schema, and the syntax definitions in schema_syntax.c).
+ *
+ */
+int samdb_msg_append_uint(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			  const char *attr_name, unsigned int v, int flags)
+{
+	return samdb_msg_append_int(sam_ldb, mem_ctx, msg, attr_name, (int)v, flags);
+}
+
+/*
+  append a (signed) int64_t element to a message
+*/
+int samdb_msg_append_int64(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			   const char *attr_name, int64_t v, int flags)
+{
+	const char *s = talloc_asprintf(mem_ctx, "%lld", (long long)v);
+	if (s == NULL) {
+		return ldb_oom(sam_ldb);
+	}
+	return ldb_msg_append_string(msg, attr_name, s, flags);
+}
+
+/*
+ * Append an unsigned int64_t (uint64_t) element to a message
+ *
+ * The issue here is that we have not yet first cast to int32_t explicitly,
+ * before we cast to an signed int to printf() into the %d or cast to a
+ * int64_t before we then cast to a long long to printf into a %lld.
+ *
+ * There are *no* unsigned integers in Active Directory LDAP, even the RID
+ * allocations and ms-DS-Secondary-KrbTgt-Number are *signed* quantities.
+ * (See the schema, and the syntax definitions in schema_syntax.c).
+ *
+ */
+int samdb_msg_append_uint64(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
+			    const char *attr_name, uint64_t v, int flags)
+{
+	return samdb_msg_append_int64(sam_ldb, mem_ctx, msg, attr_name, (int64_t)v, flags);
+}
+
+/*
   add a samr_Password element to a message
 */
 int samdb_msg_add_hash(struct ldb_context *sam_ldb, TALLOC_CTX *mem_ctx, struct ldb_message *msg,
@@ -2814,15 +2892,8 @@ NTSTATUS samdb_set_password_sid(struct ldb_context *ldb, TALLOC_CTX *mem_ctx,
 		tdo_msg->num_elements = 0;
 		TALLOC_FREE(tdo_msg->elements);
 
-		ret = ldb_msg_add_empty(tdo_msg, "trustAuthIncoming",
-					LDB_FLAG_MOD_REPLACE, NULL);
-		if (ret != LDB_SUCCESS) {
-			ldb_transaction_cancel(ldb);
-			TALLOC_FREE(frame);
-			return NT_STATUS_NO_MEMORY;
-		}
-		ret = ldb_msg_add_value(tdo_msg, "trustAuthIncoming",
-					&new_val, NULL);
+		ret = ldb_msg_append_value(tdo_msg, "trustAuthIncoming",
+					   &new_val, LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			ldb_transaction_cancel(ldb);
 			TALLOC_FREE(frame);
@@ -3187,6 +3258,7 @@ int dsdb_find_guid_by_dn(struct ldb_context *ldb,
 /*
  adds the given GUID to the given ldb_message. This value is added
  for the given attr_name (may be either "objectGUID" or "parentGUID").
+ This function is used in processing 'add' requests.
  */
 int dsdb_msg_add_guid(struct ldb_message *msg,
 		struct GUID *guid,
@@ -5656,7 +5728,8 @@ int dsdb_user_obj_set_defaults(struct ldb_context *ldb,
 }
 
 /**
- * Sets 'sAMAccountType on user object based on userAccountControl
+ * Sets 'sAMAccountType on user object based on userAccountControl.
+ * This function is used in processing both 'add' and 'modify' requests.
  * @param ldb Current ldb_context
  * @param usr_obj ldb_message representing User object
  * @param user_account_control Value for userAccountControl flags
@@ -5668,21 +5741,19 @@ int dsdb_user_obj_set_account_type(struct ldb_context *ldb, struct ldb_message *
 {
 	int ret;
 	uint32_t account_type;
-	struct ldb_message_element *el;
 
 	account_type = ds_uf2atype(user_account_control);
 	if (account_type == 0) {
 		ldb_set_errstring(ldb, "dsdb: Unrecognized account type!");
 		return LDB_ERR_UNWILLING_TO_PERFORM;
 	}
-	ret = samdb_msg_add_uint(ldb, usr_obj, usr_obj,
-				 "sAMAccountType",
-				 account_type);
+	ret = samdb_msg_add_uint_flags(ldb, usr_obj, usr_obj,
+				       "sAMAccountType",
+				       account_type,
+				       LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
-	el = ldb_msg_find_element(usr_obj, "sAMAccountType");
-	el->flags = LDB_FLAG_MOD_REPLACE;
 
 	if (account_type_p) {
 		*account_type_p = account_type;
@@ -5692,7 +5763,8 @@ int dsdb_user_obj_set_account_type(struct ldb_context *ldb, struct ldb_message *
 }
 
 /**
- * Determine and set primaryGroupID based on userAccountControl value
+ * Determine and set primaryGroupID based on userAccountControl value.
+ * This function is used in processing both 'add' and 'modify' requests.
  * @param ldb Current ldb_context
  * @param usr_obj ldb_message representing User object
  * @param user_account_control Value for userAccountControl flags
@@ -5704,17 +5776,15 @@ int dsdb_user_obj_set_primary_group_id(struct ldb_context *ldb, struct ldb_messa
 {
 	int ret;
 	uint32_t rid;
-	struct ldb_message_element *el;
 
 	rid = ds_uf2prim_group_rid(user_account_control);
 
-	ret = samdb_msg_add_uint(ldb, usr_obj, usr_obj,
-				 "primaryGroupID", rid);
+	ret = samdb_msg_add_uint_flags(ldb, usr_obj, usr_obj,
+				       "primaryGroupID", rid,
+				       LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
-	el = ldb_msg_find_element(usr_obj, "primaryGroupID");
-	el->flags = LDB_FLAG_MOD_REPLACE;
 
 	if (group_rid_p) {
 		*group_rid_p = rid;
diff --git a/source4/dsdb/samdb/ldb_modules/descriptor.c b/source4/dsdb/samdb/ldb_modules/descriptor.c
index daa08c2ebc7..4b01961dcb0 100644
--- a/source4/dsdb/samdb/ldb_modules/descriptor.c
+++ b/source4/dsdb/samdb/ldb_modules/descriptor.c
@@ -857,14 +857,8 @@ static int descriptor_modify(struct ldb_module *module, struct ldb_request *req)
 			return ldb_module_done(req, NULL, NULL, LDB_SUCCESS);
 		}
 
-		ret = ldb_msg_add_empty(msg, "nTSecurityDescriptor",
-					LDB_FLAG_MOD_REPLACE,
-					&sd_element);
-		if (ret != LDB_SUCCESS) {
-			return ldb_oom(ldb);
-		}
-		ret = ldb_msg_add_value(msg, "nTSecurityDescriptor",
-					sd, NULL);
+		ret = ldb_msg_append_value(msg, "nTSecurityDescriptor",
+					   sd, LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ldb_oom(ldb);
 		}
diff --git a/source4/dsdb/samdb/ldb_modules/objectguid.c b/source4/dsdb/samdb/ldb_modules/objectguid.c
index bc3260cf0d8..0fe995a5763 100644
--- a/source4/dsdb/samdb/ldb_modules/objectguid.c
+++ b/source4/dsdb/samdb/ldb_modules/objectguid.c
@@ -41,7 +41,6 @@
 */
 static int add_time_element(struct ldb_message *msg, const char *attr, time_t t)
 {
-	struct ldb_message_element *el;
 	char *s;
 	int ret;
 
@@ -54,16 +53,13 @@ static int add_time_element(struct ldb_message *msg, const char *attr, time_t t)
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
-	ret = ldb_msg_add_string(msg, attr, s);
+	/* always set as replace. This works because on add ops, the flag
+	   is ignored */
+	ret = ldb_msg_append_string(msg, attr, s, LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
 
-	el = ldb_msg_find_element(msg, attr);
-	/* always set as replace. This works because on add ops, the flag
-	   is ignored */
-	el->flags = LDB_FLAG_MOD_REPLACE;
-
 	return LDB_SUCCESS;
 }
 
@@ -73,23 +69,19 @@ static int add_time_element(struct ldb_message *msg, const char *attr, time_t t)
 static int add_uint64_element(struct ldb_context *ldb, struct ldb_message *msg,
 			      const char *attr, uint64_t v)
 {
-	struct ldb_message_element *el;
 	int ret;
 
 	if (ldb_msg_find_element(msg, attr) != NULL) {
 		return LDB_SUCCESS;
 	}
 
-	ret = samdb_msg_add_uint64(ldb, msg, msg, attr, v);
+	/* always set as replace. This works because on add ops, the flag
+	   is ignored */
+	ret = samdb_msg_append_uint64(ldb, msg, msg, attr, v, LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
 
-	el = ldb_msg_find_element(msg, attr);
-	/* always set as replace. This works because on add ops, the flag
-	   is ignored */
-	el->flags = LDB_FLAG_MOD_REPLACE;
-
 	return LDB_SUCCESS;
 }
 
diff --git a/source4/dsdb/samdb/ldb_modules/partition_init.c b/source4/dsdb/samdb/ldb_modules/partition_init.c
index 58c65ccedd0..484b5bffb27 100644
--- a/source4/dsdb/samdb/ldb_modules/partition_init.c
+++ b/source4/dsdb/samdb/ldb_modules/partition_init.c
@@ -742,10 +742,6 @@ int partition_create(struct ldb_module *module, struct ldb_request *req)
 		}
 		
 		mod_msg->dn = ldb_dn_new(mod_msg, ldb, DSDB_PARTITION_DN);
-		ret = ldb_msg_add_empty(mod_msg, DSDB_PARTITION_ATTR, LDB_FLAG_MOD_ADD, NULL);
-		if (ret != LDB_SUCCESS) {
-			return ret;
-		}
 		
 		casefold_dn = ldb_dn_get_casefold(dn);
 		
@@ -785,18 +781,16 @@ int partition_create(struct ldb_module *module, struct ldb_request *req)
 		}
 		partition_record = talloc_asprintf(mod_msg, "%s:%s", casefold_dn, filename);
 
-		ret = ldb_msg_add_steal_string(mod_msg, DSDB_PARTITION_ATTR, partition_record);
+		ret = ldb_msg_append_steal_string(mod_msg, DSDB_PARTITION_ATTR, partition_record,
+						  LDB_FLAG_MOD_ADD);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
 
 		if (ldb_request_get_control(req, DSDB_CONTROL_PARTIAL_REPLICA)) {
 			/* this new partition is a partial replica */
-			ret = ldb_msg_add_empty(mod_msg, "partialReplica", LDB_FLAG_MOD_ADD, NULL);
-			if (ret != LDB_SUCCESS) {
-				return ret;
-			}
-			ret = ldb_msg_add_fmt(mod_msg, "partialReplica", "%s", ldb_dn_get_linearized(dn));
+			ret = ldb_msg_append_fmt(mod_msg, LDB_FLAG_MOD_ADD,
+						 "partialReplica", "%s", ldb_dn_get_linearized(dn));
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 29ffda75c87..eec1e639856 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -3888,22 +3888,12 @@ static int replmd_rename_callback(struct ldb_request *req, struct ldb_reply *are
 				       ldb_operr(ldb));
 	}
 
-	if (ldb_msg_add_empty(msg, rdn_name, LDB_FLAG_MOD_REPLACE, NULL) != 0) {
+	if (ldb_msg_append_value(msg, rdn_name, rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		talloc_free(ares);
 		return ldb_module_done(ac->req, NULL, NULL,
 				       ldb_oom(ldb));
 	}
-	if (ldb_msg_add_value(msg, rdn_name, rdn_val, NULL) != 0) {
-		talloc_free(ares);
-		return ldb_module_done(ac->req, NULL, NULL,
-				       ldb_oom(ldb));
-	}
-	if (ldb_msg_add_empty(msg, "name", LDB_FLAG_MOD_REPLACE, NULL) != 0) {
-		talloc_free(ares);
-		return ldb_module_done(ac->req, NULL, NULL,
-				       ldb_oom(ldb));
-	}
-	if (ldb_msg_add_value(msg, "name", rdn_val, NULL) != 0) {
+	if (ldb_msg_append_value(msg, "name", rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		talloc_free(ares);
 		return ldb_module_done(ac->req, NULL, NULL,
 				       ldb_oom(ldb));
@@ -5161,16 +5151,10 @@ static int replmd_name_modify(struct replmd_replicated_request *ar,
 		goto failed;
 	}
 
-	if (ldb_msg_add_empty(msg, rdn_name, LDB_FLAG_MOD_REPLACE, NULL) != 0) {
-		goto failed;
-	}
-	if (ldb_msg_add_value(msg, rdn_name, rdn_val, NULL) != 0) {
-		goto failed;
-	}
-	if (ldb_msg_add_empty(msg, "name", LDB_FLAG_MOD_REPLACE, NULL) != 0) {
+	if (ldb_msg_append_value(msg, rdn_name, rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		goto failed;
 	}
-	if (ldb_msg_add_value(msg, "name", rdn_val, NULL) != 0) {
+	if (ldb_msg_append_value(msg, "name", rdn_val, LDB_FLAG_MOD_REPLACE) != 0) {
 		goto failed;
 	}
 
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 5fb9c195c9a..107e643e492 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -1103,14 +1103,11 @@ static int samldb_rodc_add(struct samldb_ctx *ac)
 	return LDB_ERR_OTHER;
 
 found:
-	ret = ldb_msg_add_empty(ac->msg, "msDS-SecondaryKrbTgtNumber",
-				LDB_FLAG_INTERNAL_DISABLE_VALIDATION, NULL);
-	if (ret != LDB_SUCCESS) {
-		return ldb_operr(ldb);
-	}
 
-	ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
-				 "msDS-SecondaryKrbTgtNumber", krbtgt_number);
+	ldb_msg_remove_attr(ac->msg, "msDS-SecondaryKrbTgtNumber");
+	ret = samdb_msg_append_uint(ldb, ac->msg, ac->msg,
+				    "msDS-SecondaryKrbTgtNumber", krbtgt_number,
+				    LDB_FLAG_INTERNAL_DISABLE_VALIDATION);
 	if (ret != LDB_SUCCESS) {
 		return ldb_operr(ldb);
 	}
@@ -1792,7 +1789,7 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
 	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
 	void *skip_allocate_sids = ldb_get_opaque(ldb,
 						  "skip_allocate_sids");
-	struct ldb_message_element *el, *el2;
+	struct ldb_message_element *el;
 	struct dom_sid *sid;
 	int ret;
 
@@ -1926,23 +1923,17 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
 		/* "isCriticalSystemObject" might be set */
 		if (user_account_control &
 		    (UF_SERVER_TRUST_ACCOUNT | UF_PARTIAL_SECRETS_ACCOUNT)) {
-			ret = ldb_msg_add_string(ac->msg, "isCriticalSystemObject",
-						 "TRUE");
+			ret = ldb_msg_add_string_flags(ac->msg, "isCriticalSystemObject",
+						       "TRUE", LDB_FLAG_MOD_REPLACE);
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
-			el2 = ldb_msg_find_element(ac->msg,
-						   "isCriticalSystemObject");
-			el2->flags = LDB_FLAG_MOD_REPLACE;
 		} else if (user_account_control & UF_WORKSTATION_TRUST_ACCOUNT) {
-			ret = ldb_msg_add_string(ac->msg, "isCriticalSystemObject",
-						 "FALSE");
+			ret = ldb_msg_add_string_flags(ac->msg, "isCriticalSystemObject",
+						       "FALSE", LDB_FLAG_MOD_REPLACE);
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
-			el2 = ldb_msg_find_element(ac->msg,
-						   "isCriticalSystemObject");
-			el2->flags = LDB_FLAG_MOD_REPLACE;
 		}
 
 		/* Step 1.4: "userAccountControl" -> "primaryGroupID" mapping */
@@ -2018,14 +2009,13 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
 				ldb_set_errstring(ldb, "samldb: Unrecognized account type!");
 				return LDB_ERR_UNWILLING_TO_PERFORM;
 			}
-			ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
-						 "sAMAccountType",
-						 account_type);
+			ret = samdb_msg_add_uint_flags(ldb, ac->msg, ac->msg,
+						       "sAMAccountType",
+						       account_type,
+						       LDB_FLAG_MOD_REPLACE);
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
-			el2 = ldb_msg_find_element(ac->msg, "sAMAccountType");
-			el2->flags = LDB_FLAG_MOD_REPLACE;
 		}
 		break;
 	}
@@ -2945,26 +2935,23 @@ static int samldb_user_account_control_change(struct samldb_ctx *ac)
 	}
 
 	if (old_atype != new_atype) {
-		ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
-					 "sAMAccountType", new_atype);
+		ret = samdb_msg_append_uint(ldb, ac->msg, ac->msg,
+					    "sAMAccountType", new_atype,
+					    LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el = ldb_msg_find_element(ac->msg, "sAMAccountType");
-		el->flags = LDB_FLAG_MOD_REPLACE;
 	}
 
 	/* As per MS-SAMR 3.1.1.8.10 these flags have not to be set */
 	if ((clear_uac & UF_LOCKOUT) && (old_lockoutTime != 0)) {
 		/* "lockoutTime" reset as per MS-SAMR 3.1.1.8.10 */
 		ldb_msg_remove_attr(ac->msg, "lockoutTime");
-		ret = samdb_msg_add_uint64(ldb, ac->msg, ac->msg, "lockoutTime",
-					   (NTTIME)0);
+		ret = samdb_msg_append_uint64(ldb, ac->msg, ac->msg, "lockoutTime",
+					      (NTTIME)0, LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el = ldb_msg_find_element(ac->msg, "lockoutTime");
-		el->flags = LDB_FLAG_MOD_REPLACE;
 	}
 
 	/*
@@ -2975,14 +2962,12 @@ static int samldb_user_account_control_change(struct samldb_ctx *ac)
 	 * creating the attribute.
 	 */
 	if (old_is_critical != new_is_critical || old_atype != new_atype) {
-		ret = ldb_msg_add_string(ac->msg, "isCriticalSystemObject",
-					 new_is_critical ? "TRUE": "FALSE");
+		ret = ldb_msg_append_string(ac->msg, "isCriticalSystemObject",
+					    new_is_critical ? "TRUE": "FALSE",
+					    LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el = ldb_msg_find_element(ac->msg,
-					   "isCriticalSystemObject");
-		el->flags = LDB_FLAG_MOD_REPLACE;
 	}
 
 	if (!ldb_msg_find_element(ac->msg, "primaryGroupID") &&
@@ -2995,14 +2980,12 @@ static int samldb_user_account_control_change(struct samldb_ctx *ac)
 			}
 		}
 
-		ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg,
-					 "primaryGroupID", new_pgrid);
+		ret = samdb_msg_append_uint(ldb, ac->msg, ac->msg,
+					    "primaryGroupID", new_pgrid,
+					    LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el = ldb_msg_find_element(ac->msg,
-					   "primaryGroupID");
-		el->flags = LDB_FLAG_MOD_REPLACE;
 	}
 
 	/* Propagate eventual "userAccountControl" attribute changes */
@@ -3205,13 +3188,12 @@ static int samldb_lockout_time(struct samldb_ctx *ac)
 
 	/* lockoutTime == 0 resets badPwdCount */
 	ldb_msg_remove_attr(ac->msg, "badPwdCount");
-	ret = samdb_msg_add_int(ldb, ac->msg, ac->msg,
-				"badPwdCount", 0);
+	ret = samdb_msg_append_int(ldb, ac->msg, ac->msg,
+				   "badPwdCount", 0,
+				   LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
-	el = ldb_msg_find_element(ac->msg, "badPwdCount");
-	el->flags = LDB_FLAG_MOD_REPLACE;
 
 	return LDB_SUCCESS;
 }
@@ -3309,13 +3291,11 @@ static int samldb_group_type_change(struct samldb_ctx *ac)
 		ldb_set_errstring(ldb, "samldb: Unrecognized account type!");
 		return LDB_ERR_UNWILLING_TO_PERFORM;
 	}
-	ret = samdb_msg_add_uint(ldb, ac->msg, ac->msg, "sAMAccountType",
-				 account_type);
+	ret = samdb_msg_append_uint(ldb, ac->msg, ac->msg, "sAMAccountType",
+				    account_type, LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		return ret;
 	}
-	el = ldb_msg_find_element(ac->msg, "sAMAccountType");
-	el->flags = LDB_FLAG_MOD_REPLACE;
 
 	return LDB_SUCCESS;
 }
diff --git a/source4/dsdb/samdb/ldb_modules/tombstone_reanimate.c b/source4/dsdb/samdb/ldb_modules/tombstone_reanimate.c
index 5f8911c66be..99c5955e9e7 100644
--- a/source4/dsdb/samdb/ldb_modules/tombstone_reanimate.c
+++ b/source4/dsdb/samdb/ldb_modules/tombstone_reanimate.c
@@ -294,14 +294,13 @@ static int tr_prepare_attributes(struct tr_context *ac)
 			return ldb_error(ldb, LDB_ERR_UNWILLING_TO_PERFORM,
 					 "reanimate: Unrecognized account type!");
 		}
-		ret = samdb_msg_add_uint(ldb, ac->mod_msg, ac->mod_msg,
-					 "sAMAccountType", account_type);
+		ret = samdb_msg_append_uint(ldb, ac->mod_msg, ac->mod_msg,
+					    "sAMAccountType", account_type,
+					    LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return ldb_error(ldb, LDB_ERR_OPERATIONS_ERROR,
 					 "reanimate: Failed to add sAMAccountType to restored object.");
 		}
-		el = ldb_msg_find_element(ac->mod_msg, "sAMAccountType");
-		el->flags = LDB_FLAG_MOD_REPLACE;
 
 		/* Default values set by Windows */
 		ret = samdb_find_or_add_attribute(ldb, ac->mod_msg,
@@ -324,12 +323,11 @@ static int tr_prepare_attributes(struct tr_context *ac)
 			return ret;
 		}
 
-		ret = ldb_msg_add_string(ac->mod_msg, "objectCategory", value);
+		ret = ldb_msg_append_string(ac->mod_msg, "objectCategory", value,
+					    LDB_FLAG_MOD_ADD);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
-		el = ldb_msg_find_element(ac->mod_msg, "objectCategory");
-		el->flags = LDB_FLAG_MOD_ADD;
 	}
 
 	return LDB_SUCCESS;
diff --git a/source4/nbt_server/wins/winsdb.c b/source4/nbt_server/wins/winsdb.c
index e4a7c2042ed..2a05e96bca4 100644
--- a/source4/nbt_server/wins/winsdb.c
+++ b/source4/nbt_server/wins/winsdb.c
@@ -102,13 +102,11 @@ uint64_t winsdb_set_maxVersion(struct winsdb_handle *h, uint64_t newMaxVersion)
 	msg->dn = dn;
 
 
-	ret = ldb_msg_add_empty(msg, "objectClass", LDB_FLAG_MOD_REPLACE, NULL);
+	ret = ldb_msg_append_string(msg, "objectClass", "winsMaxVersion",
+				    LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) goto failed;
-	ret = ldb_msg_add_string(msg, "objectClass", "winsMaxVersion");
-	if (ret != LDB_SUCCESS) goto failed;
-	ret = ldb_msg_add_empty(msg, "maxVersion", LDB_FLAG_MOD_REPLACE, NULL);
-	if (ret != LDB_SUCCESS) goto failed;
-	ret = ldb_msg_add_fmt(msg, "maxVersion", "%llu", (long long)newMaxVersion);
+	ret = ldb_msg_append_fmt(msg, LDB_FLAG_MOD_REPLACE,
+				 "maxVersion", "%llu", (long long)newMaxVersion);
 	if (ret != LDB_SUCCESS) goto failed;
 
 	ret = ldb_modify(wins_db, msg);
@@ -779,8 +777,7 @@ static struct ldb_message *winsdb_message(struct ldb_context *ldb,
 		ret |= ldb_msg_add_winsdb_addr(msg, rec, "address", rec->addresses[i]);
 	}
 	if (rec->registered_by) {
-		ret |= ldb_msg_add_empty(msg, "registeredBy", 0, NULL);
-		ret |= ldb_msg_add_string(msg, "registeredBy", rec->registered_by);
+		ret |= ldb_msg_append_string(msg, "registeredBy", rec->registered_by, 0);
 	}
 	if (ret != LDB_SUCCESS) goto failed;
 	return msg;
diff --git a/source4/rpc_server/lsa/dcesrv_lsa.c b/source4/rpc_server/lsa/dcesrv_lsa.c
index 15b068aec62..a165ab2b9d6 100644
--- a/source4/rpc_server/lsa/dcesrv_lsa.c
+++ b/source4/rpc_server/lsa/dcesrv_lsa.c
@@ -1778,12 +1778,7 @@ static NTSTATUS update_uint32_t_value(TALLOC_CTX *mem_ctx,
 		goto done;
 	}
 
-	ret = ldb_msg_add_empty(dest, attribute, flags, NULL);
-	if (ret != LDB_SUCCESS) {
-		return NT_STATUS_NO_MEMORY;
-	}
-
-	ret = samdb_msg_add_uint(sam_ldb, dest, dest, attribute, value);
+	ret = samdb_msg_append_uint(sam_ldb, dest, dest, attribute, value, flags);
 	if (ret != LDB_SUCCESS) {
 		return NT_STATUS_NO_MEMORY;
 	}
@@ -1874,13 +1869,7 @@ static NTSTATUS update_trust_user(TALLOC_CTX *mem_ctx,
 			continue;
 		}
 
-		ret = ldb_msg_add_empty(msg, attribute,
-					LDB_FLAG_MOD_REPLACE, NULL);
-		if (ret != LDB_SUCCESS) {
-			return NT_STATUS_NO_MEMORY;
-		}
-
-		ret = ldb_msg_add_value(msg, attribute, &v, NULL);
+		ret = ldb_msg_append_value(msg, attribute, &v, LDB_FLAG_MOD_REPLACE);
 		if (ret != LDB_SUCCESS) {
 			return NT_STATUS_NO_MEMORY;
 		}
@@ -2166,28 +2155,30 @@ static NTSTATUS setInfoTrustedDomain_base(struct dcesrv_call_state *dce_call,
 	}
 
 	if (add_incoming || del_incoming) {
-		ret = ldb_msg_add_empty(msg, "trustAuthIncoming",
-					LDB_FLAG_MOD_REPLACE, NULL);
-		if (ret != LDB_SUCCESS) {
-			return NT_STATUS_NO_MEMORY;
-		}
 		if (add_incoming) {
-			ret = ldb_msg_add_value(msg, "trustAuthIncoming",
-						&trustAuthIncoming, NULL);
+			ret = ldb_msg_append_value(msg, "trustAuthIncoming",
+						   &trustAuthIncoming, LDB_FLAG_MOD_REPLACE);
+			if (ret != LDB_SUCCESS) {
+				return NT_STATUS_NO_MEMORY;
+			}
+		} else {
+			ret = ldb_msg_add_empty(msg, "trustAuthIncoming",
+						LDB_FLAG_MOD_REPLACE, NULL);
 			if (ret != LDB_SUCCESS) {
 				return NT_STATUS_NO_MEMORY;
 			}
 		}
 	}
 	if (add_outgoing || del_outgoing) {
-		ret = ldb_msg_add_empty(msg, "trustAuthOutgoing",
-					LDB_FLAG_MOD_REPLACE, NULL);
-		if (ret != LDB_SUCCESS) {
-			return NT_STATUS_NO_MEMORY;
-		}
 		if (add_outgoing) {
-			ret = ldb_msg_add_value(msg, "trustAuthOutgoing",
-						&trustAuthOutgoing, NULL);
+			ret = ldb_msg_append_value(msg, "trustAuthOutgoing",
+						   &trustAuthOutgoing, LDB_FLAG_MOD_REPLACE);
+			if (ret != LDB_SUCCESS) {
+				return NT_STATUS_NO_MEMORY;
+			}
+		} else {
+			ret = ldb_msg_add_empty(msg, "trustAuthOutgoing",
+						LDB_FLAG_MOD_REPLACE, NULL);
 			if (ret != LDB_SUCCESS) {
 				return NT_STATUS_NO_MEMORY;
 			}
@@ -4635,14 +4626,8 @@ static NTSTATUS dcesrv_lsa_lsaRSetForestTrustInformation(struct dcesrv_call_stat
 		goto done;
 	}
 
-	ret = ldb_msg_add_empty(msg, "msDS-TrustForestTrustInfo",
-				LDB_FLAG_MOD_REPLACE, NULL);
-	if (ret != LDB_SUCCESS) {
-		status = NT_STATUS_NO_MEMORY;
-		goto done;
-	}
-	ret = ldb_msg_add_value(msg, "msDS-TrustForestTrustInfo",
-				&ft_blob, NULL);
+	ret = ldb_msg_append_value(msg, "msDS-TrustForestTrustInfo",
+				   &ft_blob, LDB_FLAG_MOD_REPLACE);
 	if (ret != LDB_SUCCESS) {
 		status = NT_STATUS_NO_MEMORY;
 		goto done;
diff --git a/source4/winbind/idmap.c b/source4/winbind/idmap.c
index c4039be473a..c6375f8357a 100644
--- a/source4/winbind/idmap.c
+++ b/source4/winbind/idmap.c
@@ -672,14 +672,8 @@ static NTSTATUS idmap_sid_to_xid(struct idmap_context *idmap_ctx,
 		vals[1].data = (uint8_t *)hwm_string;
 		vals[1].length = strlen(hwm_string);
 	} else {
-		ret = ldb_msg_add_empty(hwm_msg, "xidNumber", LDB_FLAG_MOD_ADD,
-					NULL);
-		if (ret != LDB_SUCCESS) {
-			status = NT_STATUS_NONE_MAPPED;
-			goto failed;
-		}
-
-		ret = ldb_msg_add_string(hwm_msg, "xidNumber", hwm_string);
+		ret = ldb_msg_append_string(hwm_msg, "xidNumber", hwm_string,
+					    LDB_FLAG_MOD_ADD);
 		if (ret != LDB_SUCCESS)
 		{
 			status = NT_STATUS_NONE_MAPPED;