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

Commit4bd3fad

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 parent87c23d3 commit4bd3fad

File tree

7 files changed

+195
-63
lines changed

7 files changed

+195
-63
lines changed

‎src/backend/access/common/detoast.c

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include"access/table.h"
1818
#include"access/tableam.h"
1919
#include"access/toast_internals.h"
20+
#include"common/int.h"
2021
#include"common/pg_lzcompress.h"
2122
#include"utils/expandeddatum.h"
2223
#include"utils/rel.h"
@@ -196,7 +197,8 @@ detoast_attr(struct varlena *attr)
196197
*Public entry point to get back part of a toasted value
197198
*from compression or external storage.
198199
*
199-
* Note: When slicelength is negative, return suffix of the value.
200+
* sliceoffset is where to start (zero or more)
201+
* If slicelength < 0, return everything beyond sliceoffset
200202
* ----------
201203
*/
202204
structvarlena*
@@ -206,8 +208,21 @@ detoast_attr_slice(struct varlena *attr,
206208
structvarlena*preslice;
207209
structvarlena*result;
208210
char*attrdata;
211+
int32slicelimit;
209212
int32attrsize;
210213

214+
if (sliceoffset<0)
215+
elog(ERROR,"invalid sliceoffset: %d",sliceoffset);
216+
217+
/*
218+
* Compute slicelimit = offset + length, or -1 if we must fetch all of the
219+
* value. In case of integer overflow, we must fetch all.
220+
*/
221+
if (slicelength<0)
222+
slicelimit=-1;
223+
elseif (pg_add_s32_overflow(sliceoffset,slicelength,&slicelimit))
224+
slicelength=slicelimit=-1;
225+
211226
if (VARATT_IS_EXTERNAL_ONDISK(attr))
212227
{
213228
structvaratt_externaltoast_pointer;
@@ -223,15 +238,15 @@ detoast_attr_slice(struct varlena *attr,
223238
* at least the requested part (when a prefix is requested).
224239
* Otherwise, just fetch all slices.
225240
*/
226-
if (slicelength>0&&sliceoffset >=0)
241+
if (slicelimit >=0)
227242
{
228243
int32max_size;
229244

230245
/*
231246
* Determine maximum amount of compressed data needed for a prefix
232247
* of a given length (after decompression).
233248
*/
234-
max_size=pglz_maximum_compressed_size(sliceoffset+slicelength,
249+
max_size=pglz_maximum_compressed_size(slicelimit,
235250
toast_pointer.va_extsize);
236251

237252
/*
@@ -270,8 +285,8 @@ detoast_attr_slice(struct varlena *attr,
270285
structvarlena*tmp=preslice;
271286

272287
/* Decompress enough to encompass the slice and the offset */
273-
if (slicelength>0&&sliceoffset >=0)
274-
preslice=toast_decompress_datum_slice(tmp,slicelength+sliceoffset);
288+
if (slicelimit >=0)
289+
preslice=toast_decompress_datum_slice(tmp,slicelimit);
275290
else
276291
preslice=toast_decompress_datum(tmp);
277292

@@ -297,8 +312,7 @@ detoast_attr_slice(struct varlena *attr,
297312
sliceoffset=0;
298313
slicelength=0;
299314
}
300-
301-
if (((sliceoffset+slicelength)>attrsize)||slicelength<0)
315+
elseif (slicelength<0||slicelimit>attrsize)
302316
slicelength=attrsize-sliceoffset;
303317

304318
result= (structvarlena*)palloc(slicelength+VARHDRSZ);
@@ -410,6 +424,11 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
410424
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)&&slicelength>0)
411425
slicelength=slicelength+sizeof(int32);
412426

427+
/*
428+
* Adjust length request if needed. (Note: our sole caller,
429+
* detoast_attr_slice, protects us against sliceoffset + slicelength
430+
* overflowing.)
431+
*/
413432
if (((sliceoffset+slicelength)>attrsize)||slicelength<0)
414433
slicelength=attrsize-sliceoffset;
415434

‎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
@@ -868,29 +868,38 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
868868
int32S=start;/* start position */
869869
int32S1;/* adjusted start position */
870870
int32L1;/* adjusted substring length */
871+
int32E;/* end position */
872+
873+
/*
874+
* SQL99 says S can be zero or negative, but we still must fetch from the
875+
* start of the string.
876+
*/
877+
S1=Max(S,1);
871878

872879
/* life is easy if the encoding max length is 1 */
873880
if (eml==1)
874881
{
875-
S1=Max(S,1);
876-
877882
if (length_not_specified)/* special case - get length to end of
878883
* string */
879884
L1=-1;
880-
else
885+
elseif (length<0)
886+
{
887+
/* SQL99 says to throw an error for E < S, i.e., negative length */
888+
ereport(ERROR,
889+
(errcode(ERRCODE_SUBSTRING_ERROR),
890+
errmsg("negative substring length not allowed")));
891+
L1=-1;/* silence stupider compilers */
892+
}
893+
elseif (pg_add_s32_overflow(S,length,&E))
881894
{
882-
/* end position */
883-
intE=S+length;
884-
885895
/*
886-
*A negative value for L is the only way for the end position to
887-
*be beforethestart. SQL99 says tothrow an error.
896+
*L could be large enough for S + L to overflow, in which case
897+
* thesubstring must run toend of string.
888898
*/
889-
if (E<S)
890-
ereport(ERROR,
891-
(errcode(ERRCODE_SUBSTRING_ERROR),
892-
errmsg("negative substring length not allowed")));
893-
899+
L1=-1;
900+
}
901+
else
902+
{
894903
/*
895904
* A zero or negative value for the end position can happen if the
896905
* start was negative or one. SQL99 says to return a zero-length
@@ -904,8 +913,8 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
904913

905914
/*
906915
* If the start position is past the end of the string, SQL99 says to
907-
* return a zero-length string --PG_GETARG_TEXT_P_SLICE() will do
908-
*thatfor us.Convertto zero-based starting position
916+
* return a zero-length string --DatumGetTextPSlice() will do that
917+
* for us. We need only convert S1to zero-based starting position.
909918
*/
910919
returnDatumGetTextPSlice(str,S1-1,L1);
911920
}
@@ -926,12 +935,6 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
926935
char*s;
927936
text*ret;
928937

929-
/*
930-
* if S is past the end of the string, the tuple toaster will return a
931-
* zero-length string to us
932-
*/
933-
S1=Max(S,1);
934-
935938
/*
936939
* We need to start at position zero because there is no way to know
937940
* in advance which byte offset corresponds to the supplied start
@@ -942,19 +945,24 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
942945
if (length_not_specified)/* special case - get length to end of
943946
* string */
944947
slice_size=L1=-1;
945-
else
948+
elseif (length<0)
949+
{
950+
/* SQL99 says to throw an error for E < S, i.e., negative length */
951+
ereport(ERROR,
952+
(errcode(ERRCODE_SUBSTRING_ERROR),
953+
errmsg("negative substring length not allowed")));
954+
slice_size=L1=-1;/* silence stupider compilers */
955+
}
956+
elseif (pg_add_s32_overflow(S,length,&E))
946957
{
947-
intE=S+length;
948-
949958
/*
950-
*A negative value for L is the only way for the end position to
951-
*be beforethestart. SQL99 says tothrow an error.
959+
*L could be large enough for S + L to overflow, in which case
960+
* thesubstring must run toend of string.
952961
*/
953-
if (E<S)
954-
ereport(ERROR,
955-
(errcode(ERRCODE_SUBSTRING_ERROR),
956-
errmsg("negative substring length not allowed")));
957-
962+
slice_size=L1=-1;
963+
}
964+
else
965+
{
958966
/*
959967
* A zero or negative value for the end position can happen if the
960968
* start was negative or one. SQL99 says to return a zero-length
@@ -972,8 +980,10 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
972980
/*
973981
* Total slice size in bytes can't be any longer than the start
974982
* position plus substring length times the encoding max length.
983+
* If that overflows, we can just use -1.
975984
*/
976-
slice_size= (S1+L1)*eml;
985+
if (pg_mul_s32_overflow(E,eml,&slice_size))
986+
slice_size=-1;
977987
}
978988

979989
/*
@@ -3309,9 +3319,13 @@ bytea_substring(Datum str,
33093319
intL,
33103320
boollength_not_specified)
33113321
{
3312-
intS1;/* adjusted start position */
3313-
intL1;/* adjusted substring length */
3322+
int32S1;/* adjusted start position */
3323+
int32L1;/* adjusted substring length */
3324+
int32E;/* end position */
33143325

3326+
/*
3327+
* The logic here should generally match text_substring().
3328+
*/
33153329
S1=Max(S,1);
33163330

33173331
if (length_not_specified)
@@ -3322,20 +3336,24 @@ bytea_substring(Datum str,
33223336
*/
33233337
L1=-1;
33243338
}
3325-
else
3339+
elseif (L<0)
3340+
{
3341+
/* SQL99 says to throw an error for E < S, i.e., negative length */
3342+
ereport(ERROR,
3343+
(errcode(ERRCODE_SUBSTRING_ERROR),
3344+
errmsg("negative substring length not allowed")));
3345+
L1=-1;/* silence stupider compilers */
3346+
}
3347+
elseif (pg_add_s32_overflow(S,L,&E))
33263348
{
3327-
/* end position */
3328-
intE=S+L;
3329-
33303349
/*
3331-
*A negative value for L is the only way for the end position to be
3332-
*before the start. SQL99 saystothrow an error.
3350+
*L could be large enough for S + L to overflow, in which case the
3351+
*substring must runtoend of string.
33333352
*/
3334-
if (E<S)
3335-
ereport(ERROR,
3336-
(errcode(ERRCODE_SUBSTRING_ERROR),
3337-
errmsg("negative substring length not allowed")));
3338-
3353+
L1=-1;
3354+
}
3355+
else
3356+
{
33393357
/*
33403358
* A zero or negative value for the end position can happen if the
33413359
* start was negative or one. SQL99 says to return a zero-length
@@ -3350,7 +3368,7 @@ bytea_substring(Datum str,
33503368
/*
33513369
* If the start position is past the end of the string, SQL99 says to
33523370
* return a zero-length string -- DatumGetByteaPSlice() will do that for
3353-
* us.Convertto zero-based starting position
3371+
* us. We need only convert S1to zero-based starting position.
33543372
*/
33553373
returnDatumGetByteaPSlice(str,S1-1,L1);
33563374
}

‎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));

0 commit comments

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