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(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> > po 4. 1. 2021 v 20:25 odesilatel Jerry Sievert <
> jerry(at)legitimatesounding(dot)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
>
>