aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/backend/access/common/detoast.c33
-rw-r--r--src/backend/utils/adt/varbit.c26
-rw-r--r--src/backend/utils/adt/varlena.c110
-rw-r--r--src/test/regress/expected/bit.out29
-rw-r--r--src/test/regress/expected/strings.out41
-rw-r--r--src/test/regress/sql/bit.sql8
-rw-r--r--src/test/regress/sql/strings.sql11
7 files changed, 195 insertions, 63 deletions
diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index 44c37edcbb4..3d02d2b586c 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -17,6 +17,7 @@
#include "access/table.h"
#include "access/tableam.h"
#include "access/toast_internals.h"
+#include "common/int.h"
#include "common/pg_lzcompress.h"
#include "utils/expandeddatum.h"
#include "utils/rel.h"
@@ -196,7 +197,8 @@ detoast_attr(struct varlena *attr)
* Public entry point to get back part of a toasted value
* from compression or external storage.
*
- * Note: When slicelength is negative, return suffix of the value.
+ * sliceoffset is where to start (zero or more)
+ * If slicelength < 0, return everything beyond sliceoffset
* ----------
*/
struct varlena *
@@ -206,8 +208,21 @@ detoast_attr_slice(struct varlena *attr,
struct varlena *preslice;
struct varlena *result;
char *attrdata;
+ int32 slicelimit;
int32 attrsize;
+ if (sliceoffset < 0)
+ elog(ERROR, "invalid sliceoffset: %d", sliceoffset);
+
+ /*
+ * Compute slicelimit = offset + length, or -1 if we must fetch all of the
+ * value. In case of integer overflow, we must fetch all.
+ */
+ if (slicelength < 0)
+ slicelimit = -1;
+ else if (pg_add_s32_overflow(sliceoffset, slicelength, &slicelimit))
+ slicelength = slicelimit = -1;
+
if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
struct varatt_external toast_pointer;
@@ -223,7 +238,7 @@ detoast_attr_slice(struct varlena *attr,
* at least the requested part (when a prefix is requested).
* Otherwise, just fetch all slices.
*/
- if (slicelength > 0 && sliceoffset >= 0)
+ if (slicelimit >= 0)
{
int32 max_size;
@@ -231,7 +246,7 @@ detoast_attr_slice(struct varlena *attr,
* Determine maximum amount of compressed data needed for a prefix
* of a given length (after decompression).
*/
- max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
+ max_size = pglz_maximum_compressed_size(slicelimit,
toast_pointer.va_extsize);
/*
@@ -270,8 +285,8 @@ detoast_attr_slice(struct varlena *attr,
struct varlena *tmp = preslice;
/* Decompress enough to encompass the slice and the offset */
- if (slicelength > 0 && sliceoffset >= 0)
- preslice = toast_decompress_datum_slice(tmp, slicelength + sliceoffset);
+ if (slicelimit >= 0)
+ preslice = toast_decompress_datum_slice(tmp, slicelimit);
else
preslice = toast_decompress_datum(tmp);
@@ -297,8 +312,7 @@ detoast_attr_slice(struct varlena *attr,
sliceoffset = 0;
slicelength = 0;
}
-
- if (((sliceoffset + slicelength) > attrsize) || slicelength < 0)
+ else if (slicelength < 0 || slicelimit > attrsize)
slicelength = attrsize - sliceoffset;
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
@@ -410,6 +424,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && slicelength > 0)
slicelength = slicelength + sizeof(int32);
+ /*
+ * Adjust length request if needed. (Note: our sole caller,
+ * detoast_attr_slice, protects us against sliceoffset + slicelength
+ * overflowing.)
+ */
if (((sliceoffset + slicelength) > attrsize) || slicelength < 0)
slicelength = attrsize - sliceoffset;
diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c
index f0c6a44b842..de3852045b1 100644
--- a/src/backend/utils/adt/varbit.c
+++ b/src/backend/utils/adt/varbit.c
@@ -1059,7 +1059,7 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
len,
ishift,
i;
- int e,
+ int32 e,
s1,
e1;
bits8 *r,
@@ -1072,18 +1072,24 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
{
e1 = bitlen + 1;
}
- else
+ else if (l < 0)
+ {
+ /* SQL99 says to throw an error for E < S, i.e., negative length */
+ ereport(ERROR,
+ (errcode(ERRCODE_SUBSTRING_ERROR),
+ errmsg("negative substring length not allowed")));
+ e1 = -1; /* silence stupider compilers */
+ }
+ else if (pg_add_s32_overflow(s, l, &e))
{
- e = s + l;
-
/*
- * A negative value for L is the only way for the end position to be
- * before the start. SQL99 says to throw an error.
+ * L could be large enough for S + L to overflow, in which case the
+ * substring must run to end of string.
*/
- if (e < s)
- ereport(ERROR,
- (errcode(ERRCODE_SUBSTRING_ERROR),
- errmsg("negative substring length not allowed")));
+ e1 = bitlen + 1;
+ }
+ else
+ {
e1 = Min(e, bitlen + 1);
}
if (s1 > bitlen || e1 <= s1)
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 2eaabd6231d..390da8e820d 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -851,29 +851,38 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
int32 S = start; /* start position */
int32 S1; /* adjusted start position */
int32 L1; /* adjusted substring length */
+ int32 E; /* end position */
+
+ /*
+ * SQL99 says S can be zero or negative, but we still must fetch from the
+ * start of the string.
+ */
+ S1 = Max(S, 1);
/* life is easy if the encoding max length is 1 */
if (eml == 1)
{
- S1 = Max(S, 1);
-
if (length_not_specified) /* special case - get length to end of
* string */
L1 = -1;
- else
+ else if (length < 0)
+ {
+ /* SQL99 says to throw an error for E < S, i.e., negative length */
+ ereport(ERROR,
+ (errcode(ERRCODE_SUBSTRING_ERROR),
+ errmsg("negative substring length not allowed")));
+ L1 = -1; /* silence stupider compilers */
+ }
+ else if (pg_add_s32_overflow(S, length, &E))
{
- /* end position */
- int E = S + length;
-
/*
- * A negative value for L is the only way for the end position to
- * be before the start. SQL99 says to throw an error.
+ * L could be large enough for S + L to overflow, in which case
+ * the substring must run to end of string.
*/
- if (E < S)
- ereport(ERROR,
- (errcode(ERRCODE_SUBSTRING_ERROR),
- errmsg("negative substring length not allowed")));
-
+ L1 = -1;
+ }
+ else
+ {
/*
* A zero or negative value for the end position can happen if the
* start was negative or one. SQL99 says to return a zero-length
@@ -887,8 +896,8 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
/*
* If the start position is past the end of the string, SQL99 says to
- * return a zero-length string -- PG_GETARG_TEXT_P_SLICE() will do
- * that for us. Convert to zero-based starting position
+ * return a zero-length string -- DatumGetTextPSlice() will do that
+ * for us. We need only convert S1 to zero-based starting position.
*/
return DatumGetTextPSlice(str, S1 - 1, L1);
}
@@ -910,12 +919,6 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
text *ret;
/*
- * if S is past the end of the string, the tuple toaster will return a
- * zero-length string to us
- */
- S1 = Max(S, 1);
-
- /*
* We need to start at position zero because there is no way to know
* in advance which byte offset corresponds to the supplied start
* position.
@@ -925,19 +928,24 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
if (length_not_specified) /* special case - get length to end of
* string */
slice_size = L1 = -1;
- else
+ else if (length < 0)
+ {
+ /* SQL99 says to throw an error for E < S, i.e., negative length */
+ ereport(ERROR,
+ (errcode(ERRCODE_SUBSTRING_ERROR),
+ errmsg("negative substring length not allowed")));
+ slice_size = L1 = -1; /* silence stupider compilers */
+ }
+ else if (pg_add_s32_overflow(S, length, &E))
{
- int E = S + length;
-
/*
- * A negative value for L is the only way for the end position to
- * be before the start. SQL99 says to throw an error.
+ * L could be large enough for S + L to overflow, in which case
+ * the substring must run to end of string.
*/
- if (E < S)
- ereport(ERROR,
- (errcode(ERRCODE_SUBSTRING_ERROR),
- errmsg("negative substring length not allowed")));
-
+ slice_size = L1 = -1;
+ }
+ else
+ {
/*
* A zero or negative value for the end position can happen if the
* start was negative or one. SQL99 says to return a zero-length
@@ -955,8 +963,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
/*
* Total slice size in bytes can't be any longer than the start
* position plus substring length times the encoding max length.
+ * If that overflows, we can just use -1.
*/
- slice_size = (S1 + L1) * eml;
+ if (pg_mul_s32_overflow(E, eml, &slice_size))
+ slice_size = -1;
}
/*
@@ -3278,9 +3288,13 @@ bytea_substring(Datum str,
int L,
bool length_not_specified)
{
- int S1; /* adjusted start position */
- int L1; /* adjusted substring length */
+ int32 S1; /* adjusted start position */
+ int32 L1; /* adjusted substring length */
+ int32 E; /* end position */
+ /*
+ * The logic here should generally match text_substring().
+ */
S1 = Max(S, 1);
if (length_not_specified)
@@ -3291,20 +3305,24 @@ bytea_substring(Datum str,
*/
L1 = -1;
}
- else
+ else if (L < 0)
+ {
+ /* SQL99 says to throw an error for E < S, i.e., negative length */
+ ereport(ERROR,
+ (errcode(ERRCODE_SUBSTRING_ERROR),
+ errmsg("negative substring length not allowed")));
+ L1 = -1; /* silence stupider compilers */
+ }
+ else if (pg_add_s32_overflow(S, L, &E))
{
- /* end position */
- int E = S + L;
-
/*
- * A negative value for L is the only way for the end position to be
- * before the start. SQL99 says to throw an error.
+ * L could be large enough for S + L to overflow, in which case the
+ * substring must run to end of string.
*/
- if (E < S)
- ereport(ERROR,
- (errcode(ERRCODE_SUBSTRING_ERROR),
- errmsg("negative substring length not allowed")));
-
+ L1 = -1;
+ }
+ else
+ {
/*
* A zero or negative value for the end position can happen if the
* start was negative or one. SQL99 says to return a zero-length
@@ -3319,7 +3337,7 @@ bytea_substring(Datum str,
/*
* If the start position is past the end of the string, SQL99 says to
* return a zero-length string -- DatumGetByteaPSlice() will do that for
- * us. Convert to zero-based starting position
+ * us. We need only convert S1 to zero-based starting position.
*/
return DatumGetByteaPSlice(str, S1 - 1, L1);
}
diff --git a/src/test/regress/expected/bit.out b/src/test/regress/expected/bit.out
index a1fab7ebcb0..a7f95b846d9 100644
--- a/src/test/regress/expected/bit.out
+++ b/src/test/regress/expected/bit.out
@@ -106,6 +106,35 @@ SELECT v,
01010101010 | 1010 | 01010 | 101010
(4 rows)
+-- test overflow cases
+SELECT SUBSTRING('01010101'::bit(8) FROM 2 FOR 2147483646) AS "1010101";
+ 1010101
+---------
+ 1010101
+(1 row)
+
+SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR 2147483646) AS "01010101";
+ 01010101
+----------
+ 01010101
+(1 row)
+
+SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR -2147483646) AS "error";
+ERROR: negative substring length not allowed
+SELECT SUBSTRING('01010101'::varbit FROM 2 FOR 2147483646) AS "1010101";
+ 1010101
+---------
+ 1010101
+(1 row)
+
+SELECT SUBSTRING('01010101'::varbit FROM -10 FOR 2147483646) AS "01010101";
+ 01010101
+----------
+ 01010101
+(1 row)
+
+SELECT SUBSTRING('01010101'::varbit FROM -10 FOR -2147483646) AS "error";
+ERROR: negative substring length not allowed
--- Bit operations
DROP TABLE varbit_table;
CREATE TABLE varbit_table (a BIT VARYING(16), b BIT VARYING(16));
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index 6e98d183f61..90ab198223f 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -396,6 +396,21 @@ SELECT SUBSTRING('1234567890' FROM 4 FOR 3) = '456' AS "456";
t
(1 row)
+-- test overflow cases
+SELECT SUBSTRING('string' FROM 2 FOR 2147483646) AS "tring";
+ tring
+-------
+ tring
+(1 row)
+
+SELECT SUBSTRING('string' FROM -10 FOR 2147483646) AS "string";
+ string
+--------
+ string
+(1 row)
+
+SELECT SUBSTRING('string' FROM -10 FOR -2147483646) AS "error";
+ERROR: negative substring length not allowed
-- T581 regular expression substring (with SQL's bizarre regexp syntax)
SELECT SUBSTRING('abcdefg' FROM 'a#"(b_d)#"%' FOR '#') AS "bcd";
bcd
@@ -2005,6 +2020,32 @@ SELECT repeat('Pg', -4);
(1 row)
+SELECT SUBSTRING('1234567890'::bytea FROM 3) "34567890";
+ 34567890
+----------
+ 34567890
+(1 row)
+
+SELECT SUBSTRING('1234567890'::bytea FROM 4 FOR 3) AS "456";
+ 456
+-----
+ 456
+(1 row)
+
+SELECT SUBSTRING('string'::bytea FROM 2 FOR 2147483646) AS "tring";
+ tring
+-------
+ tring
+(1 row)
+
+SELECT SUBSTRING('string'::bytea FROM -10 FOR 2147483646) AS "string";
+ string
+--------
+ string
+(1 row)
+
+SELECT SUBSTRING('string'::bytea FROM -10 FOR -2147483646) AS "error";
+ERROR: negative substring length not allowed
SELECT trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea);
btrim
-------
diff --git a/src/test/regress/sql/bit.sql b/src/test/regress/sql/bit.sql
index 7681d4ab4d0..ea01742c4aa 100644
--- a/src/test/regress/sql/bit.sql
+++ b/src/test/regress/sql/bit.sql
@@ -53,6 +53,14 @@ SELECT v,
SUBSTRING(v FROM 6) AS sub_6
FROM VARBIT_TABLE;
+-- test overflow cases
+SELECT SUBSTRING('01010101'::bit(8) FROM 2 FOR 2147483646) AS "1010101";
+SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR 2147483646) AS "01010101";
+SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR -2147483646) AS "error";
+SELECT SUBSTRING('01010101'::varbit FROM 2 FOR 2147483646) AS "1010101";
+SELECT SUBSTRING('01010101'::varbit FROM -10 FOR 2147483646) AS "01010101";
+SELECT SUBSTRING('01010101'::varbit FROM -10 FOR -2147483646) AS "error";
+
--- Bit operations
DROP TABLE varbit_table;
CREATE TABLE varbit_table (a BIT VARYING(16), b BIT VARYING(16));
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sql
index 3e89159a4fd..493547dd74f 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -131,6 +131,11 @@ SELECT SUBSTRING('1234567890' FROM 3) = '34567890' AS "34567890";
SELECT SUBSTRING('1234567890' FROM 4 FOR 3) = '456' AS "456";
+-- test overflow cases
+SELECT SUBSTRING('string' FROM 2 FOR 2147483646) AS "tring";
+SELECT SUBSTRING('string' FROM -10 FOR 2147483646) AS "string";
+SELECT SUBSTRING('string' FROM -10 FOR -2147483646) AS "error";
+
-- T581 regular expression substring (with SQL's bizarre regexp syntax)
SELECT SUBSTRING('abcdefg' FROM 'a#"(b_d)#"%' FOR '#') AS "bcd";
@@ -684,6 +689,12 @@ SELECT chr(0);
SELECT repeat('Pg', 4);
SELECT repeat('Pg', -4);
+SELECT SUBSTRING('1234567890'::bytea FROM 3) "34567890";
+SELECT SUBSTRING('1234567890'::bytea FROM 4 FOR 3) AS "456";
+SELECT SUBSTRING('string'::bytea FROM 2 FOR 2147483646) AS "tring";
+SELECT SUBSTRING('string'::bytea FROM -10 FOR 2147483646) AS "string";
+SELECT SUBSTRING('string'::bytea FROM -10 FOR -2147483646) AS "error";
+
SELECT trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea);
SELECT btrim(E'\\000trim\\000'::bytea, E'\\000'::bytea);
SELECT btrim(''::bytea, E'\\000'::bytea);