Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7c98759

Browse files
committed
Fix integer-overflow corner cases in substring() functions.
If the substring start index and length overflow when added together,substring() misbehaved, either throwing a bogus "negative substringlength" error on a case that should succeed, or failing to complain thata negative length is negative (and instead returning the whole string,in most cases). Unsurprisingly, the text, bytea, and bit variants ofthe function all had this issue. Rearrange the logic to ensure thatnegative lengths are always rejected, and add an overflow check tohandle the other case.Also install similar guards into detoast_attr_slice() (neeheap_tuple_untoast_attr_slice()), since it's far from clear thatno other code paths leading to that function could pass it valuesthat would overflow.Patch by myself and Pavel Stehule, per bug #16804 from Rafi Shamim.Back-patch to v11. While these bugs are old, the common/int.hinfrastructure for overflow-detecting arithmetic didn't exist beforecommit4d6ad31, and it doesn't seem like these misbehaviors are badenough to justify developing a standalone fix for the older branches.Discussion:https://postgr.es/m/16804-f4eeeb6c11ba71d4@postgresql.org
1 parent9a968e7 commit7c98759

File tree

7 files changed

+195
-61
lines changed

7 files changed

+195
-61
lines changed

‎src/backend/access/heap/tuptoaster.c

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include"access/tuptoaster.h"
3636
#include"access/xact.h"
3737
#include"catalog/catalog.h"
38+
#include"common/int.h"
3839
#include"common/pg_lzcompress.h"
3940
#include"miscadmin.h"
4041
#include"utils/expandeddatum.h"
@@ -252,6 +253,9 @@ heap_tuple_untoast_attr(struct varlena *attr)
252253
*
253254
*Public entry point to get back part of a toasted value
254255
*from compression or external storage.
256+
*
257+
* sliceoffset is where to start (zero or more)
258+
* If slicelength < 0, return everything beyond sliceoffset
255259
* ----------
256260
*/
257261
structvarlena*
@@ -261,8 +265,21 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
261265
structvarlena*preslice;
262266
structvarlena*result;
263267
char*attrdata;
268+
int32slicelimit;
264269
int32attrsize;
265270

271+
if (sliceoffset<0)
272+
elog(ERROR,"invalid sliceoffset: %d",sliceoffset);
273+
274+
/*
275+
* Compute slicelimit = offset + length, or -1 if we must fetch all of the
276+
* value. In case of integer overflow, we must fetch all.
277+
*/
278+
if (slicelength<0)
279+
slicelimit=-1;
280+
elseif (pg_add_s32_overflow(sliceoffset,slicelength,&slicelimit))
281+
slicelength=slicelimit=-1;
282+
266283
if (VARATT_IS_EXTERNAL_ONDISK(attr))
267284
{
268285
structvaratt_externaltoast_pointer;
@@ -303,8 +320,8 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
303320
structvarlena*tmp=preslice;
304321

305322
/* Decompress enough to encompass the slice and the offset */
306-
if (slicelength>0&&sliceoffset >=0)
307-
preslice=toast_decompress_datum_slice(tmp,slicelength+sliceoffset);
323+
if (slicelimit >=0)
324+
preslice=toast_decompress_datum_slice(tmp,slicelimit);
308325
else
309326
preslice=toast_decompress_datum(tmp);
310327

@@ -330,8 +347,7 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
330347
sliceoffset=0;
331348
slicelength=0;
332349
}
333-
334-
if (((sliceoffset+slicelength)>attrsize)||slicelength<0)
350+
elseif (slicelength<0||slicelimit>attrsize)
335351
slicelength=attrsize-sliceoffset;
336352

337353
result= (structvarlena*)palloc(slicelength+VARHDRSZ);
@@ -2086,7 +2102,12 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
20862102
length=0;
20872103
}
20882104

2089-
if (((sliceoffset+length)>attrsize)||length<0)
2105+
/*
2106+
* Adjust length request if needed. (Note: our sole caller,
2107+
* heap_tuple_untoast_attr_slice, protects us against sliceoffset + length
2108+
* overflowing.)
2109+
*/
2110+
elseif (((sliceoffset+length)>attrsize)||length<0)
20902111
length=attrsize-sliceoffset;
20912112

20922113
result= (structvarlena*)palloc(length+VARHDRSZ);

‎src/backend/utils/adt/varbit.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,7 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
10591059
len,
10601060
ishift,
10611061
i;
1062-
inte,
1062+
int32e,
10631063
s1,
10641064
e1;
10651065
bits8*r,
@@ -1072,18 +1072,24 @@ bitsubstring(VarBit *arg, int32 s, int32 l, bool length_not_specified)
10721072
{
10731073
e1=bitlen+1;
10741074
}
1075-
else
1075+
elseif (l<0)
1076+
{
1077+
/* SQL99 says to throw an error for E < S, i.e., negative length */
1078+
ereport(ERROR,
1079+
(errcode(ERRCODE_SUBSTRING_ERROR),
1080+
errmsg("negative substring length not allowed")));
1081+
e1=-1;/* silence stupider compilers */
1082+
}
1083+
elseif (pg_add_s32_overflow(s,l,&e))
10761084
{
1077-
e=s+l;
1078-
10791085
/*
1080-
*A negative value for L is the only way for the end position to be
1081-
*before the start. SQL99 saystothrow an error.
1086+
*L could be large enough for S + L to overflow, in which case the
1087+
*substring must runtoend of string.
10821088
*/
1083-
if (e<s)
1084-
ereport(ERROR,
1085-
(errcode(ERRCODE_SUBSTRING_ERROR),
1086-
errmsg("negative substring length not allowed")));
1089+
e1=bitlen+1;
1090+
}
1091+
else
1092+
{
10871093
e1=Min(e,bitlen+1);
10881094
}
10891095
if (s1>bitlen||e1 <=s1)

‎src/backend/utils/adt/varlena.c

Lines changed: 64 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -839,29 +839,38 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
839839
int32S=start;/* start position */
840840
int32S1;/* adjusted start position */
841841
int32L1;/* adjusted substring length */
842+
int32E;/* end position */
843+
844+
/*
845+
* SQL99 says S can be zero or negative, but we still must fetch from the
846+
* start of the string.
847+
*/
848+
S1=Max(S,1);
842849

843850
/* life is easy if the encoding max length is 1 */
844851
if (eml==1)
845852
{
846-
S1=Max(S,1);
847-
848853
if (length_not_specified)/* special case - get length to end of
849854
* string */
850855
L1=-1;
851-
else
856+
elseif (length<0)
857+
{
858+
/* SQL99 says to throw an error for E < S, i.e., negative length */
859+
ereport(ERROR,
860+
(errcode(ERRCODE_SUBSTRING_ERROR),
861+
errmsg("negative substring length not allowed")));
862+
L1=-1;/* silence stupider compilers */
863+
}
864+
elseif (pg_add_s32_overflow(S,length,&E))
852865
{
853-
/* end position */
854-
intE=S+length;
855-
856866
/*
857-
*A negative value for L is the only way for the end position to
858-
*be beforethestart. SQL99 says tothrow an error.
867+
*L could be large enough for S + L to overflow, in which case
868+
* thesubstring must run toend of string.
859869
*/
860-
if (E<S)
861-
ereport(ERROR,
862-
(errcode(ERRCODE_SUBSTRING_ERROR),
863-
errmsg("negative substring length not allowed")));
864-
870+
L1=-1;
871+
}
872+
else
873+
{
865874
/*
866875
* A zero or negative value for the end position can happen if the
867876
* start was negative or one. SQL99 says to return a zero-length
@@ -875,8 +884,8 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
875884

876885
/*
877886
* If the start position is past the end of the string, SQL99 says to
878-
* return a zero-length string --PG_GETARG_TEXT_P_SLICE() will do
879-
*thatfor us.Convertto zero-based starting position
887+
* return a zero-length string --DatumGetTextPSlice() will do that
888+
* for us. We need only convert S1to zero-based starting position.
880889
*/
881890
returnDatumGetTextPSlice(str,S1-1,L1);
882891
}
@@ -897,12 +906,6 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
897906
char*s;
898907
text*ret;
899908

900-
/*
901-
* if S is past the end of the string, the tuple toaster will return a
902-
* zero-length string to us
903-
*/
904-
S1=Max(S,1);
905-
906909
/*
907910
* We need to start at position zero because there is no way to know
908911
* in advance which byte offset corresponds to the supplied start
@@ -913,19 +916,24 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
913916
if (length_not_specified)/* special case - get length to end of
914917
* string */
915918
slice_size=L1=-1;
916-
else
919+
elseif (length<0)
920+
{
921+
/* SQL99 says to throw an error for E < S, i.e., negative length */
922+
ereport(ERROR,
923+
(errcode(ERRCODE_SUBSTRING_ERROR),
924+
errmsg("negative substring length not allowed")));
925+
slice_size=L1=-1;/* silence stupider compilers */
926+
}
927+
elseif (pg_add_s32_overflow(S,length,&E))
917928
{
918-
intE=S+length;
919-
920929
/*
921-
*A negative value for L is the only way for the end position to
922-
*be beforethestart. SQL99 says tothrow an error.
930+
*L could be large enough for S + L to overflow, in which case
931+
* thesubstring must run toend of string.
923932
*/
924-
if (E<S)
925-
ereport(ERROR,
926-
(errcode(ERRCODE_SUBSTRING_ERROR),
927-
errmsg("negative substring length not allowed")));
928-
933+
slice_size=L1=-1;
934+
}
935+
else
936+
{
929937
/*
930938
* A zero or negative value for the end position can happen if the
931939
* start was negative or one. SQL99 says to return a zero-length
@@ -943,8 +951,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
943951
/*
944952
* Total slice size in bytes can't be any longer than the start
945953
* position plus substring length times the encoding max length.
954+
* If that overflows, we can just use -1.
946955
*/
947-
slice_size= (S1+L1)*eml;
956+
if (pg_mul_s32_overflow(E,eml,&slice_size))
957+
slice_size=-1;
948958
}
949959

950960
/*
@@ -3246,9 +3256,13 @@ bytea_substring(Datum str,
32463256
intL,
32473257
boollength_not_specified)
32483258
{
3249-
intS1;/* adjusted start position */
3250-
intL1;/* adjusted substring length */
3259+
int32S1;/* adjusted start position */
3260+
int32L1;/* adjusted substring length */
3261+
int32E;/* end position */
32513262

3263+
/*
3264+
* The logic here should generally match text_substring().
3265+
*/
32523266
S1=Max(S,1);
32533267

32543268
if (length_not_specified)
@@ -3259,20 +3273,24 @@ bytea_substring(Datum str,
32593273
*/
32603274
L1=-1;
32613275
}
3262-
else
3276+
elseif (L<0)
3277+
{
3278+
/* SQL99 says to throw an error for E < S, i.e., negative length */
3279+
ereport(ERROR,
3280+
(errcode(ERRCODE_SUBSTRING_ERROR),
3281+
errmsg("negative substring length not allowed")));
3282+
L1=-1;/* silence stupider compilers */
3283+
}
3284+
elseif (pg_add_s32_overflow(S,L,&E))
32633285
{
3264-
/* end position */
3265-
intE=S+L;
3266-
32673286
/*
3268-
*A negative value for L is the only way for the end position to be
3269-
*before the start. SQL99 saystothrow an error.
3287+
*L could be large enough for S + L to overflow, in which case the
3288+
*substring must runtoend of string.
32703289
*/
3271-
if (E<S)
3272-
ereport(ERROR,
3273-
(errcode(ERRCODE_SUBSTRING_ERROR),
3274-
errmsg("negative substring length not allowed")));
3275-
3290+
L1=-1;
3291+
}
3292+
else
3293+
{
32763294
/*
32773295
* A zero or negative value for the end position can happen if the
32783296
* start was negative or one. SQL99 says to return a zero-length
@@ -3287,7 +3305,7 @@ bytea_substring(Datum str,
32873305
/*
32883306
* If the start position is past the end of the string, SQL99 says to
32893307
* return a zero-length string -- DatumGetByteaPSlice() will do that for
3290-
* us.Convertto zero-based starting position
3308+
* us. We need only convert S1to zero-based starting position.
32913309
*/
32923310
returnDatumGetByteaPSlice(str,S1-1,L1);
32933311
}

‎src/test/regress/expected/bit.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,35 @@ SELECT v,
106106
01010101010 | 1010 | 01010 | 101010
107107
(4 rows)
108108

109+
-- test overflow cases
110+
SELECT SUBSTRING('01010101'::bit(8) FROM 2 FOR 2147483646) AS "1010101";
111+
1010101
112+
---------
113+
1010101
114+
(1 row)
115+
116+
SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR 2147483646) AS "01010101";
117+
01010101
118+
----------
119+
01010101
120+
(1 row)
121+
122+
SELECT SUBSTRING('01010101'::bit(8) FROM -10 FOR -2147483646) AS "error";
123+
ERROR: negative substring length not allowed
124+
SELECT SUBSTRING('01010101'::varbit FROM 2 FOR 2147483646) AS "1010101";
125+
1010101
126+
---------
127+
1010101
128+
(1 row)
129+
130+
SELECT SUBSTRING('01010101'::varbit FROM -10 FOR 2147483646) AS "01010101";
131+
01010101
132+
----------
133+
01010101
134+
(1 row)
135+
136+
SELECT SUBSTRING('01010101'::varbit FROM -10 FOR -2147483646) AS "error";
137+
ERROR: negative substring length not allowed
109138
--- Bit operations
110139
DROP TABLE varbit_table;
111140
CREATE TABLE varbit_table (a BIT VARYING(16), b BIT VARYING(16));

‎src/test/regress/expected/strings.out

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,21 @@ SELECT SUBSTRING('1234567890' FROM 4 FOR 3) = '456' AS "456";
313313
t
314314
(1 row)
315315

316+
-- test overflow cases
317+
SELECT SUBSTRING('string' FROM 2 FOR 2147483646) AS "tring";
318+
tring
319+
-------
320+
tring
321+
(1 row)
322+
323+
SELECT SUBSTRING('string' FROM -10 FOR 2147483646) AS "string";
324+
string
325+
--------
326+
string
327+
(1 row)
328+
329+
SELECT SUBSTRING('string' FROM -10 FOR -2147483646) AS "error";
330+
ERROR: negative substring length not allowed
316331
-- T581 regular expression substring (with SQL's bizarre regexp syntax)
317332
SELECT SUBSTRING('abcdefg' FROM 'a#"(b_d)#"%' FOR '#') AS "bcd";
318333
bcd
@@ -1873,6 +1888,32 @@ SELECT repeat('Pg', -4);
18731888

18741889
(1 row)
18751890

1891+
SELECT SUBSTRING('1234567890'::bytea FROM 3) "34567890";
1892+
34567890
1893+
----------
1894+
34567890
1895+
(1 row)
1896+
1897+
SELECT SUBSTRING('1234567890'::bytea FROM 4 FOR 3) AS "456";
1898+
456
1899+
-----
1900+
456
1901+
(1 row)
1902+
1903+
SELECT SUBSTRING('string'::bytea FROM 2 FOR 2147483646) AS "tring";
1904+
tring
1905+
-------
1906+
tring
1907+
(1 row)
1908+
1909+
SELECT SUBSTRING('string'::bytea FROM -10 FOR 2147483646) AS "string";
1910+
string
1911+
--------
1912+
string
1913+
(1 row)
1914+
1915+
SELECT SUBSTRING('string'::bytea FROM -10 FOR -2147483646) AS "error";
1916+
ERROR: negative substring length not allowed
18761917
SELECT trim(E'\\000'::bytea from E'\\000Tom\\000'::bytea);
18771918
btrim
18781919
-------

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp