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

Commit9e7d87c

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 parentc09f688 commit9e7d87c

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
@@ -851,29 +851,38 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
851851
int32S=start;/* start position */
852852
int32S1;/* adjusted start position */
853853
int32L1;/* adjusted substring length */
854+
int32E;/* end position */
855+
856+
/*
857+
* SQL99 says S can be zero or negative, but we still must fetch from the
858+
* start of the string.
859+
*/
860+
S1=Max(S,1);
854861

855862
/* life is easy if the encoding max length is 1 */
856863
if (eml==1)
857864
{
858-
S1=Max(S,1);
859-
860865
if (length_not_specified)/* special case - get length to end of
861866
* string */
862867
L1=-1;
863-
else
868+
elseif (length<0)
869+
{
870+
/* SQL99 says to throw an error for E < S, i.e., negative length */
871+
ereport(ERROR,
872+
(errcode(ERRCODE_SUBSTRING_ERROR),
873+
errmsg("negative substring length not allowed")));
874+
L1=-1;/* silence stupider compilers */
875+
}
876+
elseif (pg_add_s32_overflow(S,length,&E))
864877
{
865-
/* end position */
866-
intE=S+length;
867-
868878
/*
869-
*A negative value for L is the only way for the end position to
870-
*be beforethestart. SQL99 says tothrow an error.
879+
*L could be large enough for S + L to overflow, in which case
880+
* thesubstring must run toend of string.
871881
*/
872-
if (E<S)
873-
ereport(ERROR,
874-
(errcode(ERRCODE_SUBSTRING_ERROR),
875-
errmsg("negative substring length not allowed")));
876-
882+
L1=-1;
883+
}
884+
else
885+
{
877886
/*
878887
* A zero or negative value for the end position can happen if the
879888
* 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)
887896

888897
/*
889898
* If the start position is past the end of the string, SQL99 says to
890-
* return a zero-length string --PG_GETARG_TEXT_P_SLICE() will do
891-
*thatfor us.Convertto zero-based starting position
899+
* return a zero-length string --DatumGetTextPSlice() will do that
900+
* for us. We need only convert S1to zero-based starting position.
892901
*/
893902
returnDatumGetTextPSlice(str,S1-1,L1);
894903
}
@@ -909,12 +918,6 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
909918
char*s;
910919
text*ret;
911920

912-
/*
913-
* if S is past the end of the string, the tuple toaster will return a
914-
* zero-length string to us
915-
*/
916-
S1=Max(S,1);
917-
918921
/*
919922
* We need to start at position zero because there is no way to know
920923
* in advance which byte offset corresponds to the supplied start
@@ -925,19 +928,24 @@ text_substring(Datum str, int32 start, int32 length, bool length_not_specified)
925928
if (length_not_specified)/* special case - get length to end of
926929
* string */
927930
slice_size=L1=-1;
928-
else
931+
elseif (length<0)
932+
{
933+
/* SQL99 says to throw an error for E < S, i.e., negative length */
934+
ereport(ERROR,
935+
(errcode(ERRCODE_SUBSTRING_ERROR),
936+
errmsg("negative substring length not allowed")));
937+
slice_size=L1=-1;/* silence stupider compilers */
938+
}
939+
elseif (pg_add_s32_overflow(S,length,&E))
929940
{
930-
intE=S+length;
931-
932941
/*
933-
*A negative value for L is the only way for the end position to
934-
*be beforethestart. SQL99 says tothrow an error.
942+
*L could be large enough for S + L to overflow, in which case
943+
* thesubstring must run toend of string.
935944
*/
936-
if (E<S)
937-
ereport(ERROR,
938-
(errcode(ERRCODE_SUBSTRING_ERROR),
939-
errmsg("negative substring length not allowed")));
940-
945+
slice_size=L1=-1;
946+
}
947+
else
948+
{
941949
/*
942950
* A zero or negative value for the end position can happen if the
943951
* 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)
955963
/*
956964
* Total slice size in bytes can't be any longer than the start
957965
* position plus substring length times the encoding max length.
966+
* If that overflows, we can just use -1.
958967
*/
959-
slice_size= (S1+L1)*eml;
968+
if (pg_mul_s32_overflow(E,eml,&slice_size))
969+
slice_size=-1;
960970
}
961971

962972
/*
@@ -3278,9 +3288,13 @@ bytea_substring(Datum str,
32783288
intL,
32793289
boollength_not_specified)
32803290
{
3281-
intS1;/* adjusted start position */
3282-
intL1;/* adjusted substring length */
3291+
int32S1;/* adjusted start position */
3292+
int32L1;/* adjusted substring length */
3293+
int32E;/* end position */
32833294

3295+
/*
3296+
* The logic here should generally match text_substring().
3297+
*/
32843298
S1=Max(S,1);
32853299

32863300
if (length_not_specified)
@@ -3291,20 +3305,24 @@ bytea_substring(Datum str,
32913305
*/
32923306
L1=-1;
32933307
}
3294-
else
3308+
elseif (L<0)
3309+
{
3310+
/* SQL99 says to throw an error for E < S, i.e., negative length */
3311+
ereport(ERROR,
3312+
(errcode(ERRCODE_SUBSTRING_ERROR),
3313+
errmsg("negative substring length not allowed")));
3314+
L1=-1;/* silence stupider compilers */
3315+
}
3316+
elseif (pg_add_s32_overflow(S,L,&E))
32953317
{
3296-
/* end position */
3297-
intE=S+L;
3298-
32993318
/*
3300-
*A negative value for L is the only way for the end position to be
3301-
*before the start. SQL99 saystothrow an error.
3319+
*L could be large enough for S + L to overflow, in which case the
3320+
*substring must runtoend of string.
33023321
*/
3303-
if (E<S)
3304-
ereport(ERROR,
3305-
(errcode(ERRCODE_SUBSTRING_ERROR),
3306-
errmsg("negative substring length not allowed")));
3307-
3322+
L1=-1;
3323+
}
3324+
else
3325+
{
33083326
/*
33093327
* A zero or negative value for the end position can happen if the
33103328
* start was negative or one. SQL99 says to return a zero-length
@@ -3319,7 +3337,7 @@ bytea_substring(Datum str,
33193337
/*
33203338
* If the start position is past the end of the string, SQL99 says to
33213339
* return a zero-length string -- DatumGetByteaPSlice() will do that for
3322-
* us.Convertto zero-based starting position
3340+
* us. We need only convert S1to zero-based starting position.
33233341
*/
33243342
returnDatumGetByteaPSlice(str,S1-1,L1);
33253343
}

‎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