pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nathan Bossart <nathandbossart@gmail.com>
To: Joseph Koshakow <koshy44@gmail.com>
Cc: jian he <jian.universality@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: Remove dependence on integer wrapping
Date: Mon, 22 Jul 2024 17:27:13 -0500
Message-ID: <Zp7cwd_6NXlHt6Ye@nathan> (raw )
In-Reply-To: <Zp7Q4erYWVjtWzQr@nathan >
References: <Zpa0m_OVw_iWYicV@nathan >
<CAAvxfHeFDyNZj+rjcuMkh+VTSYmhb_ZPpbdqZUJcjnT=X3ampA@mail.gmail.com >
<CACJufxFkTACY6MprpewMncdOpL9dpZBcAGtc6WLfnScLknS3DA@mail.gmail.com >
<CAAvxfHfeUmVMXU6883in2H-jJ+Vn2=mcSgVpsNDRTTK=OYDjoA@mail.gmail.com >
<ZpqesU46GIKUvtmS@nathan >
<Zpq0XUQ9neudWYgC@nathan >
<CAAvxfHfdPgtqFqsb=Nxa1oc6GsQ_U+k3rfneo-EjYG4bne_ksw@mail.gmail.com >
<Zp54I4WMbWW6AQnP@nathan >
<CAAvxfHdWseDnK0gLYY51fdw-_pE=5Q2=K=pToVv5jo3YNVfHoQ@mail.gmail.com >
<Zp7Q4erYWVjtWzQr@nathan >
On Mon, Jul 22, 2024 at 04:36:33PM -0500, Nathan Bossart wrote:
> Okay. I'll plan on committing v13-0002 in the next couple of days, then.
Actually, I think my concerns about prohibiting more than necessary go away
if we do the subtraction first. If "upperIndx[i] - lowerIndx[i]"
overflows, we know the array size is too big. Similarly, if adding one to
that result overflows, we again know the the array size is too big. This
appears to be how the surrounding code handles this problem (e.g.,
ReadArrayDimensions()). Thoughts?
--
nathan
From 8794c740492bd5113d405638b03518693dfeb0db Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <koshy44@gmail.com>
Date: Sat, 6 Jul 2024 14:35:00 -0400
Subject: [PATCH v14 1/1] Remove overflow from array_set_slice
This commit removes an overflow from array_set_slice that allows seting
absurd slice ranges.
---
src/backend/utils/adt/arrayfuncs.c | 9 ++++++++-
src/test/regress/expected/arrays.out | 6 ++++++
src/test/regress/sql/arrays.sql | 5 +++++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index d6641b570d..e5c7e57a5d 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -2887,7 +2887,14 @@ array_set_slice(Datum arraydatum,
errdetail("When assigning to a slice of an empty array value,"
" slice boundaries must be fully specified.")));
- dim[i] = 1 + upperIndx[i] - lowerIndx[i];
+ /* compute "upperIndx[i] - lowerIndx[i] + 1", detecting overflow */
+ if (pg_sub_s32_overflow(upperIndx[i], lowerIndx[i], &dim[i]) ||
+ pg_add_s32_overflow(dim[i], 1, &dim[i]))
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("array size exceeds the maximum allowed (%d)",
+ (int) MaxArraySize)));
+
lb[i] = lowerIndx[i];
}
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 23404982f7..7301c96bba 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2699,3 +2699,9 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
ERROR: sample size must be between 0 and 6
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
ERROR: sample size must be between 0 and 6
+-- overflow in array slicing
+CREATE TEMP TABLE arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[0:2147483647]) VALUES ('{1}');
+ERROR: array size exceeds the maximum allowed (134217727)
+INSERT INTO arroverflowtest(i[-2147483648:1]) VALUES ('{1}');
+ERROR: array size exceeds the maximum allowed (134217727)
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 50aa539fdc..d6ef059a1d 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -825,3 +825,8 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[]
SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- overflow in array slicing
+CREATE TEMP TABLE arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[0:2147483647]) VALUES ('{1}');
+INSERT INTO arroverflowtest(i[-2147483648:1]) VALUES ('{1}');
--
2.39.3 (Apple Git-146)
Attachments:
[text/plain] v14-0001-Remove-overflow-from-array_set_slice.patch (2.8K, ../Zp7cwd_6NXlHt6Ye@nathan/2-v14-0001-Remove-overflow-from-array_set_slice.patch)
download | inline diff:
From 8794c740492bd5113d405638b03518693dfeb0db Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <koshy44@gmail.com>
Date: Sat, 6 Jul 2024 14:35:00 -0400
Subject: [PATCH v14 1/1] Remove overflow from array_set_slice
This commit removes an overflow from array_set_slice that allows seting
absurd slice ranges.
---
src/backend/utils/adt/arrayfuncs.c | 9 ++++++++-
src/test/regress/expected/arrays.out | 6 ++++++
src/test/regress/sql/arrays.sql | 5 +++++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index d6641b570d..e5c7e57a5d 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -2887,7 +2887,14 @@ array_set_slice(Datum arraydatum,
errdetail("When assigning to a slice of an empty array value,"
" slice boundaries must be fully specified.")));
- dim[i] = 1 + upperIndx[i] - lowerIndx[i];
+ /* compute "upperIndx[i] - lowerIndx[i] + 1", detecting overflow */
+ if (pg_sub_s32_overflow(upperIndx[i], lowerIndx[i], &dim[i]) ||
+ pg_add_s32_overflow(dim[i], 1, &dim[i]))
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("array size exceeds the maximum allowed (%d)",
+ (int) MaxArraySize)));
+
lb[i] = lowerIndx[i];
}
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 23404982f7..7301c96bba 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2699,3 +2699,9 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
ERROR: sample size must be between 0 and 6
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
ERROR: sample size must be between 0 and 6
+-- overflow in array slicing
+CREATE TEMP TABLE arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[0:2147483647]) VALUES ('{1}');
+ERROR: array size exceeds the maximum allowed (134217727)
+INSERT INTO arroverflowtest(i[-2147483648:1]) VALUES ('{1}');
+ERROR: array size exceeds the maximum allowed (134217727)
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 50aa539fdc..d6ef059a1d 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -825,3 +825,8 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[]
SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- overflow in array slicing
+CREATE TEMP TABLE arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[0:2147483647]) VALUES ('{1}');
+INSERT INTO arroverflowtest(i[-2147483648:1]) VALUES ('{1}');
--
2.39.3 (Apple Git-146)
view thread (85+ messages) latest in thread
Message-ID: <Zp7cwd_6NXlHt6Ye@nathan>
Permalink: ../Zp7cwd_6NXlHt6Ye@nathan/
Also on: postgresql.org/message-id/Zp7cwd_6NXlHt6Ye@nathan
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, koshy44@gmail.com, jian.universality@gmail.com, exclusion@gmail.com, tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, andres@anarazel.de
Subject: Re: Remove dependence on integer wrapping
In-Reply-To: <Zp7cwd_6NXlHt6Ye@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox