]> git.ipfire.org Git - thirdparty/vim.git/commitdiff
patch 9.1.0089: qsort() comparison functions should be transitive v9.1.0089
authorChristian Brabandt <cb@256bit.org>
Fri, 9 Feb 2024 18:39:14 +0000 (19:39 +0100)
committerChristian Brabandt <cb@256bit.org>
Fri, 9 Feb 2024 18:39:14 +0000 (19:39 +0100)
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: #13980

Signed-off-by: Christian Brabandt <cb@256bit.org>
src/ex_cmds.c
src/mbyte.c
src/profiler.c
src/search.c
src/spellsuggest.c
src/version.c
src/window.c

index e7f689b83fabe88f7ebb63270567c1ff88acaab0..720e918bb49c27325dd6e76ff62283c6fc4a2389 100644 (file)
@@ -323,7 +323,7 @@ sort_compare(const void *s1, const void *s2)
     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;
index ee2834cf42f81cc299cf13248f3eb6b4ab28164e..2d18a2796a913a616ad2c2e381198062ad0b3af0 100644 (file)
@@ -5613,7 +5613,8 @@ tv_nr_compare(const void *a1, const void *a2)
     listitem_T *li1 = *(listitem_T **)a1;
     listitem_T *li2 = *(listitem_T **)a2;
 
-    return li1->li_tv.vval.v_number - li2->li_tv.vval.v_number;
+    return li1->li_tv.vval.v_number == li2->li_tv.vval.v_number ? 0 :
+       li1->li_tv.vval.v_number > li2->li_tv.vval.v_number ? 1 : -1;
 }
 
     void
index 780b958a918d437efb773b88f5ac2cbd11abda63..504d71364756dcbf58a9852199a01aaaa8abc54a 100644 (file)
@@ -287,11 +287,13 @@ profile_equal(proftime_T *tm1, proftime_T *tm2)
 profile_cmp(const proftime_T *tm1, const proftime_T *tm2)
 {
 # ifdef MSWIN
-    return (int)(tm2->QuadPart - tm1->QuadPart);
+    return tm2->QuadPart == tm1->QuadPart ? 0 :
+       tm2->QuadPart > tm1->QuadPart ? 1 : -1;
 # else
     if (tm1->tv_sec == tm2->tv_sec)
-       return tm2->tv_fsec - tm1->tv_fsec;
-    return tm2->tv_sec - tm1->tv_sec;
+       return tm2->tv_fsec == tm1->tv_fsec ? 0 :
+           tm2->tv_fsec > tm1->tv_fsec ? 1 : -1;
+    return tm2->tv_sec > tm1->tv_sec ? 1 : -1;
 # endif
 }
 
index d4baa9192ccbeed8d402cb5d6ff8cfb37294d1e2..eadbcd3d9336b13e3fa73fa0f9859381c9b5c7f3 100644 (file)
@@ -4908,7 +4908,10 @@ fuzzy_match_str_compare(const void *s1, const void *s2)
     int                idx1 = ((fuzmatch_str_T *)s1)->idx;
     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;
 }
 
 /*
@@ -4936,9 +4939,14 @@ fuzzy_match_func_compare(const void *s1, const void *s2)
     char_u     *str1 = ((fuzmatch_str_T *)s1)->str;
     char_u     *str2 = ((fuzmatch_str_T *)s2)->str;
 
-    if (*str1 != '<' && *str2 == '<') return -1;
-    if (*str1 == '<' && *str2 != '<') return 1;
-    return v1 == v2 ? (idx1 - idx2) : v1 > v2 ? -1 : 1;
+    if (*str1 != '<' && *str2 == '<')
+       return -1;
+    if (*str1 == '<' && *str2 != '<')
+       return 1;
+    if (v1 == v2)
+       return idx1 == idx2 ? 0 : idx1 > idx2 ? 1 : -1;
+    else
+       return v1 > v2 ? -1 : 1;
 }
 
 /*
index ecc0a7403d6ab968923ed217969db908352476b0..82499c0d19dfa2f4562a560805426c82760e6471 100644 (file)
@@ -3763,11 +3763,16 @@ 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;
+
+    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);
     }
index 1332f46baac1c3e617438d1015b3f867dcb77392..a4132dbebc842f452ed5a3df11c2d06bfdaa3ddc 100644 (file)
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    89,
 /**/
     88,
 /**/
index 5cb6c3cd9a1a403a53216b2b548ac377b679b178..a7d9319bdef9fcfef35c51c1611c43d525efdd4b 100644 (file)
@@ -7753,9 +7753,15 @@ frame_check_width(frame_T *topfrp, int width)
  * Simple int comparison function for use with qsort()
  */
     static int
-int_cmp(const void *a, const void *b)
-{
-    return *(const int *)a - *(const int *)b;
+int_cmp(const void *pa, const void *pb)
+{
+    const int a = *(const int *)pa;
+    const int b = *(const int *)pb;
+    if (a > b)
+       return 1;
+    if (a < b)
+       return -1;
+    return 0;
 }
 
 /*