From e06e43766500ecb4cd1031fa16cf9cbebdb222c1 Mon Sep 17 00:00:00 2001 From: Christian Brabandt Date: Fri, 9 Feb 2024 19:39:14 +0100 Subject: 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: #13980 Signed-off-by: Christian Brabandt --- src/ex_cmds.c | 2 +- src/mbyte.c | 3 ++- src/profiler.c | 8 +++++--- src/search.c | 16 ++++++++++++---- src/spellsuggest.c | 9 +++++++-- src/version.c | 2 ++ src/window.c | 12 +++++++++--- 7 files changed, 38 insertions(+), 14 deletions(-) diff --git a/src/ex_cmds.c b/src/ex_cmds.c index e7f689b83f..720e918bb4 100644 --- a/src/ex_cmds.c +++ b/src/ex_cmds.c @@ -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; diff --git a/src/mbyte.c b/src/mbyte.c index ee2834cf42..2d18a2796a 100644 --- a/src/mbyte.c +++ b/src/mbyte.c @@ -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 diff --git a/src/profiler.c b/src/profiler.c index 780b958a91..504d713647 100644 --- a/src/profiler.c +++ b/src/profiler.c @@ -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 } diff --git a/src/search.c b/src/search.c index d4baa9192c..eadbcd3d93 100644 --- a/src/search.c +++ b/src/search.c @@ -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; } /* diff --git a/src/spellsuggest.c b/src/spellsuggest.c index ecc0a7403d..82499c0d19 100644 --- a/src/spellsuggest.c +++ b/src/spellsuggest.c @@ -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); } diff --git a/src/version.c b/src/version.c index 1332f46baa..a4132dbebc 100644 --- a/src/version.c +++ b/src/version.c @@ -704,6 +704,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 89, /**/ 88, /**/ diff --git a/src/window.c b/src/window.c index 5cb6c3cd9a..a7d9319bde 100644 --- a/src/window.c +++ b/src/window.c @@ -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; } /* -- cgit v1.2.3