From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 12 Apr 2024 06:11:47 +0000 (+1200)
Subject: s4:dsdb:mod: repl_md: make message_sort transitive
X-Git-Tag: tdb-1.4.11~829
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5335f122fb551231a02a58f88f6a0aa23b5e02cb;p=thirdparty%2Fsamba.git

s4:dsdb:mod: repl_md: make message_sort transitive

Before we had (with a TODO of regret):

       if (!a1 || !a2) {
               return strcasecmp(e1->name, e2->name);
       }

so, given {name:"A", id 2}, {name:"B", NO id}, {name:"C", id 1},

 A < B by name
 B < C by name
 A > C by id

Now the sort order is always A > C > B.

This sort could have caused mysterious crashes in repl_meta_data if
the schema is out of sync.

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
index 57098f0a8b7..ea7e8a0d4a4 100644
--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
@@ -1063,12 +1063,20 @@ static int replmd_ldb_message_element_attid_sort(const struct ldb_message_elemen
 	a2 = dsdb_attribute_by_lDAPDisplayName(schema, e2->name);
 
 	/*
-	 * TODO: remove this check, we should rely on e1 and e2 having valid attribute names
-	 *       in the schema
+	 * If the elements do not have valid attribute names in the schema
+	 * (which we would prefer to think can't happen), we need to sort them
+	 * somehow. The current strategy is to put them at the end, sorted by
+	 * attribute name.
 	 */
-	if (!a1 || !a2) {
+	if (a1 == NULL && a2 == NULL) {
 		return strcasecmp(e1->name, e2->name);
 	}
+	if (a1 == NULL) {
+		return 1;
+	}
+	if (a2 == NULL) {
+		return -1;
+	}
 	if (a1->attributeID_id == a2->attributeID_id) {
 		return 0;
 	}