Re: BUG #16804: substring() function returns "negative substring length" error when using a large length argument

I forgot to include this in my original bug report, but the bytea substring
function (which has a separate implementation in varlena.c) is also
affected.

> SELECT encode(substring(decode('010203', 'hex'), 2, 2147483646), 'hex');
2021-01-04 16:50:22.020 EST [85734] ERROR:  negative substring length not
allowed
2021-01-04 16:50:22.020 EST [85734] STATEMENT:  SELECT
encode(substring(decode('010203', 'hex'), 2, 2147483646), 'hex')
negative substring length not allowed


On Mon, Jan 4, 2021 at 4:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Pavel Stehule <pavel.stehule@gmail.com> writes:
> > po 4. 1. 2021 v 20:25 odesilatel Jerry Sievert <
> jerry@legitimatesounding.com>
> > napsal:
> >> The original bug report is one less than +2147483647, and thus should
> be a
> >> valid value, no?
>
> > yes, so the implementation patch is really correct.
>
> I agree that this is a bug, and that what we should do in case of integer
> overflow is return all the rest of the string.  But this patch doesn't
> really get the job done, because you haven't accounted for *negative*
> overflow, ie, S+L < INT_MIN.  I think the best way to fix that is to
> explicitly check for L < 0 rather than trying to wait till after the
> addition.
>
> Looking around, I notice that there's an unprotected multiplication
> further down.  Also, having seen this, I feel very uncomfortable about
> the fact that detoast_attr_slice and cohorts aren't guarding against
> the same sort of integer overflow.  I don't think this is the only
> caller that might pass out-of-range values.
>
> In short, I think we need something more like the attached.
>
>                         regards, tom lane
>
>