vim-patch:9.1.0089: qsort() comparison functions should be transitive

Problem:  qsort() comparison functions should be transitive
Solution: Do not subtract values, but rather use explicit comparisons

Improve qsort() comparison functions

There has been a recent report on qsort() causing out-of-bounds read &
write in glibc for non transitive comparison functions
https://www.qualys.com/2024/01/30/qsort.txt

Even so the bug is in glibc's implementation of the qsort() algorithm,
it's bad style to just use substraction for the comparison functions,
which may cause overflow issues and as hinted at in OpenBSD's manual
page for qsort(): "It is almost always an error to use subtraction to
compute the return value of the comparison function."

So check the qsort() comparison functions and change them to be safe.

closes: vim/vim#13980

https://github.com/vim/vim/commit/e06e43766500ecb4cd1031fa16cf9cbebdb222c1

Co-authored-by: Christian Brabandt <cb@256bit.org>

5 files changed, 29 insertions, 13 deletions

diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index e265dc139a..70146a8602 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -406,18 +406,16 @@ static int sort_compare(const void *s1, const void *s2)
   // number.
   if (sort_nr) {
     if (l1.st_u.num.is_number != l2.st_u.num.is_number) {
-      result = l1.st_u.num.is_number - l2.st_u.num.is_number;
+      result = l1.st_u.num.is_number > l2.st_u.num.is_number ? 1 : -1;
     } else {
       result = l1.st_u.num.value == l2.st_u.num.value
                ? 0
-               : l1.st_u.num.value > l2.st_u.num.value
-               ? 1
-               : -1;
+               : l1.st_u.num.value > l2.st_u.num.value ? 1 : -1;
     }
   } else if (sort_flt) {
     result = l1.st_u.value_flt == l2.st_u.value_flt
-             ? 0 : l1.st_u.value_flt > l2.st_u.value_flt
-             ? 1 : -1;
+             ? 0
+             : l1.st_u.value_flt > l2.st_u.value_flt ? 1 : -1;
   } else {
     // We need to copy one line into "sortbuf1", because there is no
     // guarantee that the first pointer becomes invalid when obtaining the
diff --git a/src/nvim/mbyte.c b/src/nvim/mbyte.c
index 8583b236c7..fd353d8a67 100644
--- a/src/nvim/mbyte.c
+++ b/src/nvim/mbyte.c
@@ -2792,8 +2792,10 @@ static int tv_nr_compare(const void *a1, const void *a2)
 {
   const listitem_T *const li1 = tv_list_first(*(const list_T **)a1);
   const listitem_T *const li2 = tv_list_first(*(const list_T **)a2);
+  const varnumber_T n1 = TV_LIST_ITEM_TV(li1)->vval.v_number;
+  const varnumber_T n2 = TV_LIST_ITEM_TV(li2)->vval.v_number;
 
-  return (int)(TV_LIST_ITEM_TV(li1)->vval.v_number - TV_LIST_ITEM_TV(li2)->vval.v_number);
+  return n1 == n2 ? 0 : n1 > n2 ? 1 : -1;
 }
 
 /// "setcellwidths()" function
diff --git a/src/nvim/search.c b/src/nvim/search.c
index 273a924876..f666b07c72 100644
--- a/src/nvim/search.c
+++ b/src/nvim/search.c
@@ -3436,7 +3436,11 @@ static int fuzzy_match_str_compare(const void *const s1, const void *const s2)
   const int idx1 = ((fuzmatch_str_T *)s1)->idx;
   const int idx2 = ((fuzmatch_str_T *)s2)->idx;
 
-  return v1 == v2 ? (idx1 - idx2) : v1 > v2 ? -1 : 1;
+  if (v1 == v2) {
+    return idx1 == idx2 ? 0 : idx1 > idx2 ? 1 : -1;
+  } else {
+    return v1 > v2 ? -1 : 1;
+  }
 }
 
 /// Sort fuzzy matches by score
@@ -3465,7 +3469,11 @@ static int fuzzy_match_func_compare(const void *const s1, const void *const s2)
   if (*str1 == '<' && *str2 != '<') {
     return 1;
   }
-  return v1 == v2 ? (idx1 - idx2) : v1 > v2 ? -1 : 1;
+  if (v1 == v2) {
+    return idx1 == idx2 ? 0 : idx1 > idx2 ? 1 : -1;
+  } else {
+    return v1 > v2 ? -1 : 1;
+  }
 }
 
 /// Sort fuzzy matches of function names by score.
diff --git a/src/nvim/spellsuggest.c b/src/nvim/spellsuggest.c
index 887ad3a62a..5c0e295f88 100644
--- a/src/nvim/spellsuggest.c
+++ b/src/nvim/spellsuggest.c
@@ -3221,10 +3221,10 @@ static int sug_compare(const void *s1, const void *s2)
 {
   suggest_T *p1 = (suggest_T *)s1;
   suggest_T *p2 = (suggest_T *)s2;
-  int n = p1->st_score - p2->st_score;
+  int n = p1->st_score == p2->st_score ? 0 : p1->st_score > p2->st_score ? 1 : -1;
 
   if (n == 0) {
-    n = p1->st_altscore - p2->st_altscore;
+    n = p1->st_altscore == p2->st_altscore ? 0 : p1->st_altscore > p2->st_altscore ? 1 : -1;
     if (n == 0) {
       n = STRICMP(p1->st_word, p2->st_word);
     }
diff --git a/src/nvim/window.c b/src/nvim/window.c
index a188d75000..15bd1212ad 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -7359,9 +7359,17 @@ static bool frame_check_width(const frame_T *topfrp, int width)
 }
 
 /// Simple int comparison function for use with qsort()
-static int int_cmp(const void *a, const void *b)
+static int int_cmp(const void *pa, const void *pb)
 {
-  return *(const int *)a - *(const int *)b;
+  const int a = *(const int *)pa;
+  const int b = *(const int *)pb;
+  if (a > b) {
+    return 1;
+  }
+  if (a < b) {
+    return -1;
+  }
+  return 0;
 }
 
 /// Handle setting 'colorcolumn' or 'textwidth' in window "wp".