c8-stream-10
imports/c8-stream-10/postgresql-10.23-3.module+el8.9.0+20831+78489779
MSVSphere Packaging Team
1 year ago
parent
1e759ff0a5
commit
8ae7ad4c71
@ -0,0 +1,306 @@
|
||||
From c48008f599fd0e3bca2923ff5b40b559ae1e4e2e Mon Sep 17 00:00:00 2001
|
||||
From: Tom Lane <tgl@sss.pgh.pa.us>
|
||||
Date: Mon, 6 Nov 2023 10:56:43 -0500
|
||||
Subject: [PATCH] Detect integer overflow while computing new array dimensions.
|
||||
|
||||
array_set_element() and related functions allow an array to be
|
||||
enlarged by assigning to subscripts outside the current array bounds.
|
||||
While these places were careful to check that the new bounds are
|
||||
allowable, they neglected to consider the risk of integer overflow
|
||||
in computing the new bounds. In edge cases, we could compute new
|
||||
bounds that are invalid but get past the subsequent checks,
|
||||
allowing bad things to happen. Memory stomps that are potentially
|
||||
exploitable for arbitrary code execution are possible, and so is
|
||||
disclosure of server memory.
|
||||
|
||||
To fix, perform the hazardous computations using overflow-detecting
|
||||
arithmetic routines, which fortunately exist in all still-supported
|
||||
branches.
|
||||
|
||||
The test cases added for this generate (after patching) errors that
|
||||
mention the value of MaxArraySize, which is platform-dependent.
|
||||
Rather than introduce multiple expected-files, use psql's VERBOSITY
|
||||
parameter to suppress the printing of the message text. v11 psql
|
||||
lacks that parameter, so omit the tests in that branch.
|
||||
|
||||
Our thanks to Pedro Gallegos for reporting this problem.
|
||||
|
||||
Security: CVE-2023-5869
|
||||
---
|
||||
src/backend/utils/adt/arrayfuncs.c | 85 +++++++++++++++++++++++-------
|
||||
src/backend/utils/adt/arrayutils.c | 6 ---
|
||||
src/include/common/int.h | 69 ++++++++++++++++++++++++
|
||||
src/include/utils/array.h | 6 +++
|
||||
4 files changed, 142 insertions(+), 24 deletions(-)
|
||||
create mode 100644 src/include/common/int.h
|
||||
|
||||
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
|
||||
index 553c517..7363893 100644
|
||||
--- a/src/backend/utils/adt/arrayfuncs.c
|
||||
+++ b/src/backend/utils/adt/arrayfuncs.c
|
||||
@@ -22,6 +22,7 @@
|
||||
|
||||
#include "access/htup_details.h"
|
||||
#include "catalog/pg_type.h"
|
||||
+#include "common/int.h"
|
||||
#include "funcapi.h"
|
||||
#include "libpq/pqformat.h"
|
||||
#include "utils/array.h"
|
||||
@@ -2309,22 +2310,38 @@ array_set_element(Datum arraydatum,
|
||||
addedbefore = addedafter = 0;
|
||||
|
||||
/*
|
||||
- * Check subscripts
|
||||
+ * Check subscripts. We assume the existing subscripts passed
|
||||
+ * ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
|
||||
+ * overflow. But we must beware of other overflows in our calculations of
|
||||
+ * new dim[] values.
|
||||
*/
|
||||
if (ndim == 1)
|
||||
{
|
||||
if (indx[0] < lb[0])
|
||||
{
|
||||
- addedbefore = lb[0] - indx[0];
|
||||
- dim[0] += addedbefore;
|
||||
+ /* addedbefore = lb[0] - indx[0]; */
|
||||
+ /* dim[0] += addedbefore; */
|
||||
+ if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
lb[0] = indx[0];
|
||||
if (addedbefore > 1)
|
||||
newhasnulls = true; /* will insert nulls */
|
||||
}
|
||||
if (indx[0] >= (dim[0] + lb[0]))
|
||||
{
|
||||
- addedafter = indx[0] - (dim[0] + lb[0]) + 1;
|
||||
- dim[0] += addedafter;
|
||||
+ /* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
|
||||
+ /* dim[0] += addedafter; */
|
||||
+ if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
|
||||
+ pg_add_s32_overflow(addedafter, 1, &addedafter) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
if (addedafter > 1)
|
||||
newhasnulls = true; /* will insert nulls */
|
||||
}
|
||||
@@ -2568,14 +2585,23 @@ array_set_element_expanded(Datum arraydatum,
|
||||
addedbefore = addedafter = 0;
|
||||
|
||||
/*
|
||||
- * Check subscripts (this logic matches original array_set_element)
|
||||
+ * Check subscripts (this logic must match array_set_element). We assume
|
||||
+ * the existing subscripts passed ArrayCheckBounds, so that dim[i] + lb[i]
|
||||
+ * can be computed without overflow. But we must beware of other
|
||||
+ * overflows in our calculations of new dim[] values.
|
||||
*/
|
||||
if (ndim == 1)
|
||||
{
|
||||
if (indx[0] < lb[0])
|
||||
{
|
||||
- addedbefore = lb[0] - indx[0];
|
||||
- dim[0] += addedbefore;
|
||||
+ /* addedbefore = lb[0] - indx[0]; */
|
||||
+ /* dim[0] += addedbefore; */
|
||||
+ if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
lb[0] = indx[0];
|
||||
dimschanged = true;
|
||||
if (addedbefore > 1)
|
||||
@@ -2583,8 +2609,15 @@ array_set_element_expanded(Datum arraydatum,
|
||||
}
|
||||
if (indx[0] >= (dim[0] + lb[0]))
|
||||
{
|
||||
- addedafter = indx[0] - (dim[0] + lb[0]) + 1;
|
||||
- dim[0] += addedafter;
|
||||
+ /* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
|
||||
+ /* dim[0] += addedafter; */
|
||||
+ if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
|
||||
+ pg_add_s32_overflow(addedafter, 1, &addedafter) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
dimschanged = true;
|
||||
if (addedafter > 1)
|
||||
newhasnulls = true; /* will insert nulls */
|
||||
@@ -2866,7 +2899,10 @@ array_set_slice(Datum arraydatum,
|
||||
addedbefore = addedafter = 0;
|
||||
|
||||
/*
|
||||
- * Check subscripts
|
||||
+ * Check subscripts. We assume the existing subscripts passed
|
||||
+ * ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
|
||||
+ * overflow. But we must beware of other overflows in our calculations of
|
||||
+ * new dim[] values.
|
||||
*/
|
||||
if (ndim == 1)
|
||||
{
|
||||
@@ -2881,18 +2917,31 @@ array_set_slice(Datum arraydatum,
|
||||
errmsg("upper bound cannot be less than lower bound")));
|
||||
if (lowerIndx[0] < lb[0])
|
||||
{
|
||||
- if (upperIndx[0] < lb[0] - 1)
|
||||
- newhasnulls = true; /* will insert nulls */
|
||||
- addedbefore = lb[0] - lowerIndx[0];
|
||||
- dim[0] += addedbefore;
|
||||
+ /* addedbefore = lb[0] - lowerIndx[0]; */
|
||||
+ /* dim[0] += addedbefore; */
|
||||
+ if (pg_sub_s32_overflow(lb[0], lowerIndx[0], &addedbefore) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
lb[0] = lowerIndx[0];
|
||||
+ if (addedbefore > 1)
|
||||
+ newhasnulls = true; /* will insert nulls */
|
||||
}
|
||||
if (upperIndx[0] >= (dim[0] + lb[0]))
|
||||
{
|
||||
- if (lowerIndx[0] > (dim[0] + lb[0]))
|
||||
+ /* addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1; */
|
||||
+ /* dim[0] += addedafter; */
|
||||
+ if (pg_sub_s32_overflow(upperIndx[0], dim[0] + lb[0], &addedafter) ||
|
||||
+ pg_add_s32_overflow(addedafter, 1, &addedafter) ||
|
||||
+ pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
|
||||
+ ereport(ERROR,
|
||||
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
|
||||
+ errmsg("array size exceeds the maximum allowed (%d)",
|
||||
+ (int) MaxArraySize)));
|
||||
+ if (addedafter > 1)
|
||||
newhasnulls = true; /* will insert nulls */
|
||||
- addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1;
|
||||
- dim[0] += addedafter;
|
||||
}
|
||||
}
|
||||
else
|
||||
diff --git a/src/backend/utils/adt/arrayutils.c b/src/backend/utils/adt/arrayutils.c
|
||||
index f7c6a51..eb5f2a0 100644
|
||||
--- a/src/backend/utils/adt/arrayutils.c
|
||||
+++ b/src/backend/utils/adt/arrayutils.c
|
||||
@@ -63,10 +63,6 @@ ArrayGetOffset0(int n, const int *tup, const int *scale)
|
||||
* This must do overflow checking, since it is used to validate that a user
|
||||
* dimensionality request doesn't overflow what we can handle.
|
||||
*
|
||||
- * We limit array sizes to at most about a quarter billion elements,
|
||||
- * so that it's not necessary to check for overflow in quite so many
|
||||
- * places --- for instance when palloc'ing Datum arrays.
|
||||
- *
|
||||
* The multiplication overflow check only works on machines that have int64
|
||||
* arithmetic, but that is nearly all platforms these days, and doing check
|
||||
* divides for those that don't seems way too expensive.
|
||||
@@ -77,8 +73,6 @@ ArrayGetNItems(int ndim, const int *dims)
|
||||
int32 ret;
|
||||
int i;
|
||||
|
||||
-#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
|
||||
-
|
||||
if (ndim <= 0)
|
||||
return 0;
|
||||
ret = 1;
|
||||
diff --git a/src/include/common/int.h b/src/include/common/int.h
|
||||
new file mode 100644
|
||||
index 0000000..4235586
|
||||
--- /dev/null
|
||||
+++ b/src/include/common/int.h
|
||||
@@ -0,0 +1,69 @@
|
||||
+/*-------------------------------------------------------------------------
|
||||
+ *
|
||||
+ * int.h
|
||||
+ * Routines to perform integer math, while checking for overflows.
|
||||
+ *
|
||||
+ * The routines in this file are intended to be well defined C, without
|
||||
+ * relying on compiler flags like -fwrapv.
|
||||
+ *
|
||||
+ * To reduce the overhead of these routines try to use compiler intrinsics
|
||||
+ * where available. That's not that important for the 16, 32 bit cases, but
|
||||
+ * the 64 bit cases can be considerably faster with intrinsics. In case no
|
||||
+ * intrinsics are available 128 bit math is used where available.
|
||||
+ *
|
||||
+ * Copyright (c) 2017-2018, PostgreSQL Global Development Group
|
||||
+ *
|
||||
+ * src/include/common/int.h
|
||||
+ *
|
||||
+ *-------------------------------------------------------------------------
|
||||
+ */
|
||||
+#ifndef COMMON_INT_H
|
||||
+#define COMMON_INT_H
|
||||
+
|
||||
+/*
|
||||
+ * If a + b overflows, return true, otherwise store the result of a + b into
|
||||
+ * *result. The content of *result is implementation defined in case of
|
||||
+ * overflow.
|
||||
+ */
|
||||
+static inline bool
|
||||
+pg_add_s32_overflow(int32 a, int32 b, int32 *result)
|
||||
+{
|
||||
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
|
||||
+ return __builtin_add_overflow(a, b, result);
|
||||
+#else
|
||||
+ int64 res = (int64) a + (int64) b;
|
||||
+
|
||||
+ if (res > PG_INT32_MAX || res < PG_INT32_MIN)
|
||||
+ {
|
||||
+ *result = 0x5EED; /* to avoid spurious warnings */
|
||||
+ return true;
|
||||
+ }
|
||||
+ *result = (int32) res;
|
||||
+ return false;
|
||||
+#endif
|
||||
+}
|
||||
+
|
||||
+/*
|
||||
+ * If a - b overflows, return true, otherwise store the result of a - b into
|
||||
+ * *result. The content of *result is implementation defined in case of
|
||||
+ * overflow.
|
||||
+ */
|
||||
+static inline bool
|
||||
+pg_sub_s32_overflow(int32 a, int32 b, int32 *result)
|
||||
+{
|
||||
+#if defined(HAVE__BUILTIN_OP_OVERFLOW)
|
||||
+ return __builtin_sub_overflow(a, b, result);
|
||||
+#else
|
||||
+ int64 res = (int64) a - (int64) b;
|
||||
+
|
||||
+ if (res > PG_INT32_MAX || res < PG_INT32_MIN)
|
||||
+ {
|
||||
+ *result = 0x5EED; /* to avoid spurious warnings */
|
||||
+ return true;
|
||||
+ }
|
||||
+ *result = (int32) res;
|
||||
+ return false;
|
||||
+#endif
|
||||
+}
|
||||
+
|
||||
+#endif /* COMMON_INT_H */
|
||||
diff --git a/src/include/utils/array.h b/src/include/utils/array.h
|
||||
index 905f6b0..2d96834 100644
|
||||
--- a/src/include/utils/array.h
|
||||
+++ b/src/include/utils/array.h
|
||||
@@ -64,6 +64,12 @@
|
||||
#include "fmgr.h"
|
||||
#include "utils/expandeddatum.h"
|
||||
|
||||
+/*
|
||||
+ * Maximum number of elements in an array. We limit this to at most about a
|
||||
+ * quarter billion elements, so that it's not necessary to check for overflow
|
||||
+ * in quite so many places --- for instance when palloc'ing Datum arrays.
|
||||
+ */
|
||||
+#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
|
||||
|
||||
/*
|
||||
* Arrays are varlena objects, so must meet the varlena convention that
|
||||
--
|
||||
2.43.0
|
||||
Loading…
Reference in new issue