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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Jerry Sievert <jerry(at)legitimatesounding(dot)com>, rafiss(at)gmail(dot)com, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: BUG #16804: substring() function returns "negative substring length" error when using a large length argument
Date: 2021-01-04 21:28:01
Message-ID: 3219376.1609795681@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

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

Attachment Content-Type Size
more-overflow-checks-for-substrings-2.patch text/x-diff 8.9 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Rafi Shamim 2021-01-04 21:52:13 Re: BUG #16804: substring() function returns "negative substring length" error when using a large length argument
Previous Message Pavel Stehule 2021-01-04 19:30:59 Re: BUG #16804: substring() function returns "negative substring length" error when using a large length argument