Skip to content

Commit e24daa9

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 d3d1e25 commit e24daa9

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"
@@ -2358,22 +2359,38 @@ array_set_element(Datum arraydatum,
23582359
addedbefore = addedafter = 0;
23592360

23602361
/*
2361-
* Check subscripts
2362+
* Check subscripts. We assume the existing subscripts passed
2363+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2364+
* overflow. But we must beware of other overflows in our calculations of
2365+
* new dim[] values.
23622366
*/
23632367
if (ndim == 1)
23642368
{
23652369
if (indx[0] < lb[0])
23662370
{
2367-
addedbefore = lb[0] - indx[0];
2368-
dim[0] += addedbefore;
2371+
/* addedbefore = lb[0] - indx[0]; */
2372+
/* dim[0] += addedbefore; */
2373+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2374+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2375+
ereport(ERROR,
2376+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2377+
errmsg("array size exceeds the maximum allowed (%d)",
2378+
(int) MaxArraySize)));
23692379
lb[0] = indx[0];
23702380
if (addedbefore > 1)
23712381
newhasnulls = true; /* will insert nulls */
23722382
}
23732383
if (indx[0] >= (dim[0] + lb[0]))
23742384
{
2375-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2376-
dim[0] += addedafter;
2385+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2386+
/* dim[0] += addedafter; */
2387+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2388+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2389+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2390+
ereport(ERROR,
2391+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2392+
errmsg("array size exceeds the maximum allowed (%d)",
2393+
(int) MaxArraySize)));
23772394
if (addedafter > 1)
23782395
newhasnulls = true; /* will insert nulls */
23792396
}
@@ -2619,23 +2636,39 @@ array_set_element_expanded(Datum arraydatum,
26192636
addedbefore = addedafter = 0;
26202637

26212638
/*
2622-
* Check subscripts (this logic matches original array_set_element)
2639+
* Check subscripts (this logic must match array_set_element). We assume
2640+
* the existing subscripts passed ArrayCheckBounds, so that dim[i] + lb[i]
2641+
* can be computed without overflow. But we must beware of other
2642+
* overflows in our calculations of new dim[] values.
26232643
*/
26242644
if (ndim == 1)
26252645
{
26262646
if (indx[0] < lb[0])
26272647
{
2628-
addedbefore = lb[0] - indx[0];
2629-
dim[0] += addedbefore;
2648+
/* addedbefore = lb[0] - indx[0]; */
2649+
/* dim[0] += addedbefore; */
2650+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2651+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2652+
ereport(ERROR,
2653+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2654+
errmsg("array size exceeds the maximum allowed (%d)",
2655+
(int) MaxArraySize)));
26302656
lb[0] = indx[0];
26312657
dimschanged = true;
26322658
if (addedbefore > 1)
26332659
newhasnulls = true; /* will insert nulls */
26342660
}
26352661
if (indx[0] >= (dim[0] + lb[0]))
26362662
{
2637-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2638-
dim[0] += addedafter;
2663+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2664+
/* dim[0] += addedafter; */
2665+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2666+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2667+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2668+
ereport(ERROR,
2669+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2670+
errmsg("array size exceeds the maximum allowed (%d)",
2671+
(int) MaxArraySize)));
26392672
dimschanged = true;
26402673
if (addedafter > 1)
26412674
newhasnulls = true; /* will insert nulls */
@@ -2918,7 +2951,10 @@ array_set_slice(Datum arraydatum,
29182951
addedbefore = addedafter = 0;
29192952

29202953
/*
2921-
* Check subscripts
2954+
* Check subscripts. We assume the existing subscripts passed
2955+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2956+
* overflow. But we must beware of other overflows in our calculations of
2957+
* new dim[] values.
29222958
*/
29232959
if (ndim == 1)
29242960
{
@@ -2933,18 +2969,31 @@ array_set_slice(Datum arraydatum,
29332969
errmsg("upper bound cannot be less than lower bound")));
29342970
if (lowerIndx[0] < lb[0])
29352971
{
2936-
if (upperIndx[0] < lb[0] - 1)
2937-
newhasnulls = true; /* will insert nulls */
2938-
addedbefore = lb[0] - lowerIndx[0];
2939-
dim[0] += addedbefore;
2972+
/* addedbefore = lb[0] - lowerIndx[0]; */
2973+
/* dim[0] += addedbefore; */
2974+
if (pg_sub_s32_overflow(lb[0], lowerIndx[0], &addedbefore) ||
2975+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2976+
ereport(ERROR,
2977+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2978+
errmsg("array size exceeds the maximum allowed (%d)",
2979+
(int) MaxArraySize)));
29402980
lb[0] = lowerIndx[0];
2981+
if (addedbefore > 1)
2982+
newhasnulls = true; /* will insert nulls */
29412983
}
29422984
if (upperIndx[0] >= (dim[0] + lb[0]))
29432985
{
2944-
if (lowerIndx[0] > (dim[0] + lb[0]))
2986+
/* addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1; */
2987+
/* dim[0] += addedafter; */
2988+
if (pg_sub_s32_overflow(upperIndx[0], dim[0] + lb[0], &addedafter) ||
2989+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2990+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2991+
ereport(ERROR,
2992+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2993+
errmsg("array size exceeds the maximum allowed (%d)",
2994+
(int) MaxArraySize)));
2995+
if (addedafter > 1)
29452996
newhasnulls = true; /* will insert nulls */
2946-
addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1;
2947-
dim[0] += addedafter;
29482997
}
29492998
}
29502999
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.
@@ -88,8 +84,6 @@ ArrayGetNItemsSafe(int ndim, const int *dims, struct Node *escontext)
8884
int32 ret;
8985
int i;
9086

91-
#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
92-
9387
if (ndim <= 0)
9488
return 0;
9589
ret = 1;

‎src/include/utils/array.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@ struct ExprContext;
7474
*/
7575
#define MAXDIM 6
7676

77+
/*
78+
* Maximum number of elements in an array. We limit this to at most about a
79+
* quarter billion elements, so that it's not necessary to check for overflow
80+
* in quite so many places --- for instance when palloc'ing Datum arrays.
81+
*/
82+
#define MaxArraySize ((Size) (MaxAllocSize / sizeof(Datum)))
83+
7784
/*
7885
* Arrays are varlena objects, so must meet the varlena convention that
7986
* 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
@@ -1418,6 +1418,23 @@ insert into arr_pk_tbl(pk, f1[1:2]) values (1, '{6,7,8}') on conflict (pk)
14181418
-- then you didn't get an indexscan plan, and something is busted.
14191419
reset enable_seqscan;
14201420
reset enable_bitmapscan;
1421+
-- test subscript overflow detection
1422+
-- The normal error message includes a platform-dependent limit,
1423+
-- so suppress it to avoid needing multiple expected-files.
1424+
\set VERBOSITY sqlstate
1425+
insert into arr_pk_tbl values(10, '[-2147483648:-2147483647]={1,2}');
1426+
update arr_pk_tbl set f1[2147483647] = 42 where pk = 10;
1427+
ERROR: 54000
1428+
update arr_pk_tbl set f1[2147483646:2147483647] = array[4,2] where pk = 10;
1429+
ERROR: 54000
1430+
-- also exercise the expanded-array case
1431+
do $$ declare a int[];
1432+
begin
1433+
a := '[-2147483648:-2147483647]={1,2}'::int[];
1434+
a[2147483647] := 42;
1435+
end $$;
1436+
ERROR: 54000
1437+
\set VERBOSITY default
14211438
-- test [not] (like|ilike) (any|all) (...)
14221439
select 'foo' like any (array['%a', '%o']); -- t
14231440
?column?

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

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

441+
-- test subscript overflow detection
442+
443+
-- The normal error message includes a platform-dependent limit,
444+
-- so suppress it to avoid needing multiple expected-files.
445+
\set VERBOSITY sqlstate
446+
447+
insert into arr_pk_tbl values(10, '[-2147483648:-2147483647]={1,2}');
448+
update arr_pk_tbl set f1[2147483647] = 42 where pk = 10;
449+
update arr_pk_tbl set f1[2147483646:2147483647] = array[4,2] where pk = 10;
450+
451+
-- also exercise the expanded-array case
452+
do $$ declare a int[];
453+
begin
454+
a := '[-2147483648:-2147483647]={1,2}'::int[];
455+
a[2147483647] := 42;
456+
end $$;
457+
458+
\set VERBOSITY default
459+
441460
-- test [not] (like|ilike) (any|all) (...)
442461
select 'foo' like any (array['%a', '%o']); -- t
443462
select 'foo' like any (array['%a', '%b']); -- f

0 commit comments

Comments
 (0)