Skip to content

Commit d267cea

Browse files
tglsfdctglsfdc
committed
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
1 parent e911afd commit d267cea

File tree

5 files changed

+110
-24
lines changed

5 files changed

+110
-24
lines changed

‎src/backend/utils/adt/arrayfuncs.c

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "access/htup_details.h"
2121
#include "catalog/pg_type.h"
22+
#include "common/int.h"
2223
#include "funcapi.h"
2324
#include "libpq/pqformat.h"
2425
#include "nodes/nodeFuncs.h"
@@ -2310,22 +2311,38 @@ array_set_element(Datum arraydatum,
23102311
addedbefore = addedafter = 0;
23112312

23122313
/*
2313-
* Check subscripts
2314+
* Check subscripts. We assume the existing subscripts passed
2315+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2316+
* overflow. But we must beware of other overflows in our calculations of
2317+
* new dim[] values.
23142318
*/
23152319
if (ndim == 1)
23162320
{
23172321
if (indx[0] < lb[0])
23182322
{
2319-
addedbefore = lb[0] - indx[0];
2320-
dim[0] += addedbefore;
2323+
/* addedbefore = lb[0] - indx[0]; */
2324+
/* dim[0] += addedbefore; */
2325+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2326+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2327+
ereport(ERROR,
2328+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2329+
errmsg("array size exceeds the maximum allowed (%d)",
2330+
(int) MaxArraySize)));
23212331
lb[0] = indx[0];
23222332
if (addedbefore > 1)
23232333
newhasnulls = true; /* will insert nulls */
23242334
}
23252335
if (indx[0] >= (dim[0] + lb[0]))
23262336
{
2327-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2328-
dim[0] += addedafter;
2337+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2338+
/* dim[0] += addedafter; */
2339+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2340+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2341+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2342+
ereport(ERROR,
2343+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2344+
errmsg("array size exceeds the maximum allowed (%d)",
2345+
(int) MaxArraySize)));
23292346
if (addedafter > 1)
23302347
newhasnulls = true; /* will insert nulls */
23312348
}
@@ -2568,23 +2585,39 @@ array_set_element_expanded(Datum arraydatum,
25682585
addedbefore = addedafter = 0;
25692586

25702587
/*
2571-
* Check subscripts (this logic matches original array_set_element)
2588+
* Check subscripts (this logic must match array_set_element). We assume
2589+
* the existing subscripts passed ArrayCheckBounds, so that dim[i] + lb[i]
2590+
* can be computed without overflow. But we must beware of other
2591+
* overflows in our calculations of new dim[] values.
25722592
*/
25732593
if (ndim == 1)
25742594
{
25752595
if (indx[0] < lb[0])
25762596
{
2577-
addedbefore = lb[0] - indx[0];
2578-
dim[0] += addedbefore;
2597+
/* addedbefore = lb[0] - indx[0]; */
2598+
/* dim[0] += addedbefore; */
2599+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2600+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2601+
ereport(ERROR,
2602+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2603+
errmsg("array size exceeds the maximum allowed (%d)",
2604+
(int) MaxArraySize)));
25792605
lb[0] = indx[0];
25802606
dimschanged = true;
25812607
if (addedbefore > 1)
25822608
newhasnulls = true; /* will insert nulls */
25832609
}
25842610
if (indx[0] >= (dim[0] + lb[0]))
25852611
{
2586-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2587-
dim[0] += addedafter;
2612+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2613+
/* dim[0] += addedafter; */
2614+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2615+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2616+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2617+
ereport(ERROR,
2618+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2619+
errmsg("array size exceeds the maximum allowed (%d)",
2620+
(int) MaxArraySize)));
25882621
dimschanged = true;
25892622
if (addedafter > 1)
25902623
newhasnulls = true; /* will insert nulls */
@@ -2866,7 +2899,10 @@ array_set_slice(Datum arraydatum,
28662899
addedbefore = addedafter = 0;
28672900

28682901
/*
2869-
* Check subscripts
2902+
* Check subscripts. We assume the existing subscripts passed
2903+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2904+
* overflow. But we must beware of other overflows in our calculations of
2905+
* new dim[] values.
28702906
*/
28712907
if (ndim == 1)
28722908
{
@@ -2881,18 +2917,31 @@ array_set_slice(Datum arraydatum,
28812917
errmsg("upper bound cannot be less than lower bound")));
28822918
if (lowerIndx[0] < lb[0])
28832919
{
2884-
if (upperIndx[0] < lb[0] - 1)
2885-
newhasnulls = true; /* will insert nulls */
2886-
addedbefore = lb[0] - lowerIndx[0];
2887-
dim[0] += addedbefore;
2920+
/* addedbefore = lb[0] - lowerIndx[0]; */
2921+
/* dim[0] += addedbefore; */
2922+
if (pg_sub_s32_overflow(lb[0], lowerIndx[0], &addedbefore) ||
2923+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2924+
ereport(ERROR,
2925+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2926+
errmsg("array size exceeds the maximum allowed (%d)",
2927+
(int) MaxArraySize)));
28882928
lb[0] = lowerIndx[0];
2929+
if (addedbefore > 1)
2930+
newhasnulls = true; /* will insert nulls */
28892931
}
28902932
if (upperIndx[0] >= (dim[0] + lb[0]))
28912933
{
2892-
if (lowerIndx[0] > (dim[0] + lb[0]))
2934+
/* addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1; */
2935+
/* dim[0] += addedafter; */
2936+
if (pg_sub_s32_overflow(upperIndx[0], dim[0] + lb[0], &addedafter) ||
2937+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2938+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2939+
ereport(ERROR,
2940+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2941+
errmsg("array size exceeds the maximum allowed (%d)",
2942+
(int) MaxArraySize)));
2943+
if (addedafter > 1)
28932944
newhasnulls = true; /* will insert nulls */
2894-
addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1;
2895-
dim[0] += addedafter;
28962945
}
28972946
}
28982947
else

‎src/backend/utils/adt/arrayutils.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ ArrayGetOffset0(int n, const int *tup, const int *scale)
6464
* This must do overflow checking, since it is used to validate that a user
6565
* dimensionality request doesn't overflow what we can handle.
6666
*
67-
* We limit array sizes to at most about a quarter billion elements,
68-
* so that it's not necessary to check for overflow in quite so many
69-
* places --- for instance when palloc'ing Datum arrays.
70-
*
7167
* The multiplication overflow check only works on machines that have int64
7268
* arithmetic, but that is nearly all platforms these days, and doing check
7369
* divides for those that don't seems way too expensive.
@@ -78,8 +74,6 @@ ArrayGetNItems(int ndim, const int *dims)
7874
int32 ret;
7975
int i;
8076

81-
#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
82-
8377
if (ndim <= 0)
8478
return 0;
8579
ret = 1;

‎src/include/utils/array.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,13 @@ struct ExprState;
6969
struct ExprContext;
7070

7171

72+
/*
73+
* Maximum number of elements in an array. We limit this to at most about a
74+
* quarter billion elements, so that it's not necessary to check for overflow
75+
* in quite so many places --- for instance when palloc'ing Datum arrays.
76+
*/
77+
#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
78+
7279
/*
7380
* Arrays are varlena objects, so must meet the varlena convention that
7481
* the first int32 of the object contains the total object size in bytes.

‎src/test/regress/expected/arrays.out

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,6 +1347,23 @@ insert into arr_pk_tbl(pk, f1[1:2]) values (1, '{6,7,8}') on conflict (pk)
13471347
-- then you didn't get an indexscan plan, and something is busted.
13481348
reset enable_seqscan;
13491349
reset enable_bitmapscan;
1350+
-- test subscript overflow detection
1351+
-- The normal error message includes a platform-dependent limit,
1352+
-- so suppress it to avoid needing multiple expected-files.
1353+
\set VERBOSITY sqlstate
1354+
insert into arr_pk_tbl values(10, '[-2147483648:-2147483647]={1,2}');
1355+
update arr_pk_tbl set f1[2147483647] = 42 where pk = 10;
1356+
ERROR: 54000
1357+
update arr_pk_tbl set f1[2147483646:2147483647] = array[4,2] where pk = 10;
1358+
ERROR: 54000
1359+
-- also exercise the expanded-array case
1360+
do $$ declare a int[];
1361+
begin
1362+
a := '[-2147483648:-2147483647]={1,2}'::int[];
1363+
a[2147483647] := 42;
1364+
end $$;
1365+
ERROR: 54000
1366+
\set VERBOSITY default
13501367
-- test [not] (like|ilike) (any|all) (...)
13511368
select 'foo' like any (array['%a', '%o']); -- t
13521369
?column?

‎src/test/regress/sql/arrays.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,25 @@ insert into arr_pk_tbl(pk, f1[1:2]) values (1, '{6,7,8}') on conflict (pk)
407407
reset enable_seqscan;
408408
reset enable_bitmapscan;
409409

410+
-- test subscript overflow detection
411+
412+
-- The normal error message includes a platform-dependent limit,
413+
-- so suppress it to avoid needing multiple expected-files.
414+
\set VERBOSITY sqlstate
415+
416+
insert into arr_pk_tbl values(10, '[-2147483648:-2147483647]={1,2}');
417+
update arr_pk_tbl set f1[2147483647] = 42 where pk = 10;
418+
update arr_pk_tbl set f1[2147483646:2147483647] = array[4,2] where pk = 10;
419+
420+
-- also exercise the expanded-array case
421+
do $$ declare a int[];
422+
begin
423+
a := '[-2147483648:-2147483647]={1,2}'::int[];
424+
a[2147483647] := 42;
425+
end $$;
426+
427+
\set VERBOSITY default
428+
410429
-- test [not] (like|ilike) (any|all) (...)
411430
select 'foo' like any (array['%a', '%o']); -- t
412431
select 'foo' like any (array['%a', '%b']); -- f

0 commit comments

Comments
 (0)