Hi hackers,
There is a bug in glibc's qsort() algorithm that runs the risk of creating
an out-of-bounds error if the comparison function is not transitive, for
example, if subtraction is used so that it can create an overflow.
See
https://packetstormsecurity.com/files/176931/glibc-qsort-Out-Of-Bounds-Read-Write.html
I checked the existing functions in the latest version of Postgres source
code and most are safe, but there were a few ones that could lead to
overflow. I do not know if these can actually lead to problems, but better
safe than sorry, so I created a patch to fix those few cases and add a
comment to one case that was not clear that it could not overflow.
Best wishes,
Mats Kindahl, Timescale
From 83e2f14ab259f568034b07c2f99e34c6dff2c7b5 Mon Sep 17 00:00:00 2001
From: Mats Kindahl <m...@timescale.com>
Date: Tue, 6 Feb 2024 14:53:53 +0100
Subject: Ensure comparison functions are transitive
There are a few comparison functions to qsort() that are non-transitive
because they can cause an overflow. Fix these functions to instead use
normal comparisons and return -1, 0, or 1 explicitly.
---
src/backend/access/spgist/spgtextproc.c | 6 +++++-
src/backend/utils/cache/relcache.c | 2 ++
src/bin/pg_upgrade/function.c | 10 +++++++---
src/bin/psql/crosstabview.c | 8 +++++++-
4 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/spgist/spgtextproc.c b/src/backend/access/spgist/spgtextproc.c
index b8fd0c2ad8..d0a2b4e6e1 100644
--- a/src/backend/access/spgist/spgtextproc.c
+++ b/src/backend/access/spgist/spgtextproc.c
@@ -325,7 +325,11 @@ cmpNodePtr(const void *a, const void *b)
const spgNodePtr *aa = (const spgNodePtr *) a;
const spgNodePtr *bb = (const spgNodePtr *) b;
- return aa->c - bb->c;
+ if (aa->c > bb->c)
+ return 1;
+ if (aa->c < bb->c)
+ return -1;
+ return 0;
}
Datum
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index ac106b40e3..42e4359c7b 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -4520,6 +4520,8 @@ AttrDefaultCmp(const void *a, const void *b)
const AttrDefault *ada = (const AttrDefault *) a;
const AttrDefault *adb = (const AttrDefault *) b;
+ /* Cannot overflow because standard conversions will convert to int and
+ * sizeof(int16) < sizeof(int) */
return ada->adnum - adb->adnum;
}
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index af998f74d3..c82ae11016 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -32,14 +32,18 @@ library_name_compare(const void *p1, const void *p2)
int slen1 = strlen(str1);
int slen2 = strlen(str2);
int cmp = strcmp(str1, str2);
+ int lhsdb = ((const LibraryInfo *) p1)->dbnum;
+ int rhsdb = ((const LibraryInfo *) p2)->dbnum;
if (slen1 != slen2)
return slen1 - slen2;
if (cmp != 0)
return cmp;
- else
- return ((const LibraryInfo *) p1)->dbnum -
- ((const LibraryInfo *) p2)->dbnum;
+ if (lhsdb < rhsdb)
+ return -1;
+ if (lhsdb > rhsdb)
+ return 1;
+ return 0;
}
diff --git a/src/bin/psql/crosstabview.c b/src/bin/psql/crosstabview.c
index c6116b7238..948ed9b1fe 100644
--- a/src/bin/psql/crosstabview.c
+++ b/src/bin/psql/crosstabview.c
@@ -709,5 +709,11 @@ pivotFieldCompare(const void *a, const void *b)
static int
rankCompare(const void *a, const void *b)
{
- return *((const int *) a) - *((const int *) b);
+ const int lhs = *(const int *) a;
+ const int rhs = *(const int *) b;
+ if (lhs < rhs)
+ return -1;
+ if (lhs > rhs)
+ return 1;
+ return 0;
}
--
2.34.1
