diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index 52ce062c52..da878a7348 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -35,6 +35,7 @@ #include "access/tuptoaster.h" #include "access/xact.h" #include "catalog/catalog.h" +#include "common/int.h" #include "common/pg_lzcompress.h" #include "miscadmin.h" #include "utils/expandeddatum.h" @@ -252,6 +253,9 @@ heap_tuple_untoast_attr(struct varlena *attr) * * Public entry point to get back part of a toasted value * from compression or external storage. + * + * sliceoffset is where to start (zero or more) + * If slicelength < 0, return everything beyond sliceoffset * ---------- */ struct varlena * @@ -261,8 +265,21 @@ heap_tuple_untoast_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; @@ -326,8 +343,7 @@ heap_tuple_untoast_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); @@ -2093,7 +2109,12 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length) length = 0; } - if (((sliceoffset + length) > attrsize) || length < 0) + /* + * Adjust length request if needed. (Note: our sole caller, + * heap_tuple_untoast_attr_slice, protects us against sliceoffset + length + * overflowing.) + */ + else if (((sliceoffset + length) > attrsize) || length < 0) length = attrsize - sliceoffset; result = (struct varlena *) palloc(length + VARHDRSZ); diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c index f6fbd0d2f2..044f9e2831 100644 --- a/src/backend/utils/adt/varbit.c +++ b/src/backend/utils/adt/varbit.c @@ -1049,7 +1049,7 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified) len, ishift, i; - int e, + int32 e, s1, e1; bits8 *r, @@ -1062,18 +1062,24 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified) { e1 = bitlen + 1; } + 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)) + { + /* + * L could be large enough for S + L to overflow, in which case the + * substring must run to end of string. + */ + e1 = bitlen + 1; + } else { - 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. - */ - if (e < s) - ereport(ERROR, - (errcode(ERRCODE_SUBSTRING_ERROR), - errmsg("negative substring length not allowed"))); 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 6d42bca1ea..bb8bce5304 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -817,29 +817,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 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)) + { + /* + * L could be large enough for S + L to overflow, in which case + * the substring must run to end of string. + */ + L1 = -1; + } else { - /* 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. - */ - if (E < S) - ereport(ERROR, - (errcode(ERRCODE_SUBSTRING_ERROR), - errmsg("negative substring length not allowed"))); - /* * 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 @@ -853,8 +862,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); } @@ -875,12 +884,6 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified) char *s; 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 @@ -891,19 +894,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 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)) + { + /* + * L could be large enough for S + L to overflow, in which case + * the substring must run to end of string. + */ + slice_size = L1 = -1; + } else { - 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. - */ - if (E < S) - ereport(ERROR, - (errcode(ERRCODE_SUBSTRING_ERROR), - errmsg("negative substring length not allowed"))); - /* * 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 @@ -921,8 +929,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; } /* @@ -2884,9 +2894,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) @@ -2897,20 +2911,24 @@ bytea_substring(Datum str, */ L1 = -1; } + 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)) + { + /* + * L could be large enough for S + L to overflow, in which case the + * substring must run to end of string. + */ + L1 = -1; + } else { - /* 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. - */ - if (E < S) - ereport(ERROR, - (errcode(ERRCODE_SUBSTRING_ERROR), - errmsg("negative substring length not allowed"))); - /* * 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 @@ -2925,7 +2943,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 a1fab7ebcb..a7f95b846d 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 ae7b1e15e8..6fbaf152cd 100644 --- a/src/test/regress/expected/strings.out +++ b/src/test/regress/expected/strings.out @@ -313,7 +313,22 @@ SELECT SUBSTRING('1234567890' FROM 4 FOR 3) = '456' AS "456"; t (1 row) --- T581 regular expression substring (with SQL99's bizarre regexp syntax) +-- 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 ----- @@ -1788,6 +1803,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 7681d4ab4d..ea01742c4a 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 9433ec1550..8eb6bf30af 100644 --- a/src/test/regress/sql/strings.sql +++ b/src/test/regress/sql/strings.sql @@ -110,7 +110,12 @@ SELECT SUBSTRING('1234567890' FROM 3) = '34567890' AS "34567890"; SELECT SUBSTRING('1234567890' FROM 4 FOR 3) = '456' AS "456"; --- T581 regular expression substring (with SQL99's bizarre regexp syntax) +-- 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"; -- No match should return NULL @@ -613,6 +618,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);