util:tsort.h: add a macro for safely comparing numbers

In many places we use `return a - b;` in a comparison function. This can
be problematic if the comparison is used in a sort, as `a - b` is not
guaranteed to do what we expect. For example:

* if a and b are 2s-complement ints, a is INT_MIN and b is INT_MAX, then
  a - b = 1, which is wrong.

* if a and b are 64 bit pointers, a - b could wrap around many times in
  a cmp function returning 32 bit ints. (We do this often).

The issue is not just that a sort could go haywire.
Due to a bug in glibc, this could result in out-of-bounds access:

https://www.openwall.com/lists/oss-security/2024/01/30/7

(We have replicated this bug in ldb_qsort).

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/lib/util/tsort.h b/lib/util/tsort.h
index 811d6cd2f77c734c8c9c28acd9d661e4d0cd9978..18e82d6c9fe374e7ed45c34a878cd35c86e12ddb 100644 (file)
--- a/lib/util/tsort.h
+++ b/lib/util/tsort.h
@@ -37,4 +37,23 @@ do { \
 } while (0)
 #endif
 
+
+#ifndef NUMERIC_CMP
+/*
+ * NUMERIC_CMP is a safe replacement for `a - b` in comparison
+ * functions. It will work on integers, pointers, and floats.
+ *
+ * Rather than
+ *
+ *      return a - b;
+ *
+ * use
+ *
+ *     return NUMERIC_CMP(a, b);
+ *
+ * and you won't have any troubles if a - b would overflow.
+ */
+#define NUMERIC_CMP(a, b) (((a) > (b)) - ((a) < (b)))
+#endif
+
 #endif