Skip to content

Commit 18b5851

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 3b0776f commit 18b5851

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"
@@ -2326,22 +2327,38 @@ array_set_element(Datum arraydatum,
23262327
addedbefore = addedafter = 0;
23272328

23282329
/*
2329-
* Check subscripts
2330+
* Check subscripts. We assume the existing subscripts passed
2331+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2332+
* overflow. But we must beware of other overflows in our calculations of
2333+
* new dim[] values.
23302334
*/
23312335
if (ndim == 1)
23322336
{
23332337
if (indx[0] < lb[0])
23342338
{
2335-
addedbefore = lb[0] - indx[0];
2336-
dim[0] += addedbefore;
2339+
/* addedbefore = lb[0] - indx[0]; */
2340+
/* dim[0] += addedbefore; */
2341+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2342+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2343+
ereport(ERROR,
2344+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2345+
errmsg("array size exceeds the maximum allowed (%d)",
2346+
(int) MaxArraySize)));
23372347
lb[0] = indx[0];
23382348
if (addedbefore > 1)
23392349
newhasnulls = true; /* will insert nulls */
23402350
}
23412351
if (indx[0] >= (dim[0] + lb[0]))
23422352
{
2343-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2344-
dim[0] += addedafter;
2353+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2354+
/* dim[0] += addedafter; */
2355+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2356+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2357+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2358+
ereport(ERROR,
2359+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2360+
errmsg("array size exceeds the maximum allowed (%d)",
2361+
(int) MaxArraySize)));
23452362
if (addedafter > 1)
23462363
newhasnulls = true; /* will insert nulls */
23472364
}
@@ -2587,23 +2604,39 @@ array_set_element_expanded(Datum arraydatum,
25872604
addedbefore = addedafter = 0;
25882605

25892606
/*
2590-
* Check subscripts (this logic matches original array_set_element)
2607+
* Check subscripts (this logic must match array_set_element). We assume
2608+
* the existing subscripts passed ArrayCheckBounds, so that dim[i] + lb[i]
2609+
* can be computed without overflow. But we must beware of other
2610+
* overflows in our calculations of new dim[] values.
25912611
*/
25922612
if (ndim == 1)
25932613
{
25942614
if (indx[0] < lb[0])
25952615
{
2596-
addedbefore = lb[0] - indx[0];
2597-
dim[0] += addedbefore;
2616+
/* addedbefore = lb[0] - indx[0]; */
2617+
/* dim[0] += addedbefore; */
2618+
if (pg_sub_s32_overflow(lb[0], indx[0], &addedbefore) ||
2619+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2620+
ereport(ERROR,
2621+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2622+
errmsg("array size exceeds the maximum allowed (%d)",
2623+
(int) MaxArraySize)));
25982624
lb[0] = indx[0];
25992625
dimschanged = true;
26002626
if (addedbefore > 1)
26012627
newhasnulls = true; /* will insert nulls */
26022628
}
26032629
if (indx[0] >= (dim[0] + lb[0]))
26042630
{
2605-
addedafter = indx[0] - (dim[0] + lb[0]) + 1;
2606-
dim[0] += addedafter;
2631+
/* addedafter = indx[0] - (dim[0] + lb[0]) + 1; */
2632+
/* dim[0] += addedafter; */
2633+
if (pg_sub_s32_overflow(indx[0], dim[0] + lb[0], &addedafter) ||
2634+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2635+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2636+
ereport(ERROR,
2637+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2638+
errmsg("array size exceeds the maximum allowed (%d)",
2639+
(int) MaxArraySize)));
26072640
dimschanged = true;
26082641
if (addedafter > 1)
26092642
newhasnulls = true; /* will insert nulls */
@@ -2886,7 +2919,10 @@ array_set_slice(Datum arraydatum,
28862919
addedbefore = addedafter = 0;
28872920

28882921
/*
2889-
* Check subscripts
2922+
* Check subscripts. We assume the existing subscripts passed
2923+
* ArrayCheckBounds, so that dim[i] + lb[i] can be computed without
2924+
* overflow. But we must beware of other overflows in our calculations of
2925+
* new dim[] values.
28902926
*/
28912927
if (ndim == 1)
28922928
{
@@ -2901,18 +2937,31 @@ array_set_slice(Datum arraydatum,
29012937
errmsg("upper bound cannot be less than lower bound")));
29022938
if (lowerIndx[0] < lb[0])
29032939
{
2904-
if (upperIndx[0] < lb[0] - 1)
2905-
newhasnulls = true; /* will insert nulls */
2906-
addedbefore = lb[0] - lowerIndx[0];
2907-
dim[0] += addedbefore;
2940+
/* addedbefore = lb[0] - lowerIndx[0]; */
2941+
/* dim[0] += addedbefore; */
2942+
if (pg_sub_s32_overflow(lb[0], lowerIndx[0], &addedbefore) ||
2943+
pg_add_s32_overflow(dim[0], addedbefore, &dim[0]))
2944+
ereport(ERROR,
2945+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2946+
errmsg("array size exceeds the maximum allowed (%d)",
2947+
(int) MaxArraySize)));
29082948
lb[0] = lowerIndx[0];
2949+
if (addedbefore > 1)
2950+
newhasnulls = true; /* will insert nulls */
29092951
}
29102952
if (upperIndx[0] >= (dim[0] + lb[0]))
29112953
{
2912-
if (lowerIndx[0] > (dim[0] + lb[0]))
2954+
/* addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1; */
2955+
/* dim[0] += addedafter; */
2956+
if (pg_sub_s32_overflow(upperIndx[0], dim[0] + lb[0], &addedafter) ||
2957+
pg_add_s32_overflow(addedafter, 1, &addedafter) ||
2958+
pg_add_s32_overflow(dim[0], addedafter, &dim[0]))
2959+
ereport(ERROR,
2960+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2961+
errmsg("array size exceeds the maximum allowed (%d)",
2962+
(int) MaxArraySize)));
2963+
if (addedafter > 1)
29132964
newhasnulls = true; /* will insert nulls */
2914-
addedafter = upperIndx[0] - (dim[0] + lb[0]) + 1;
2915-
dim[0] += addedafter;
29162965
}
29172966
}
29182967
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)