BUG #18200: Undefined behaviour in interval_div

Started by PG Bug reporting formover 2 years ago5 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

The following bug has been logged on the website:

Bug reference: 18200
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 16.1
Operating system: Ubuntu 22.04
Description:

The following query:
SELECT interval '12000000 months' / 0.001;

triggers an ubsan-detected error:
timestamp.c:3408:18: runtime error: 1.2e+10 is outside the range of
representable values of type 'int'

Without ubsan the result is:
-178956970 years -8 mons -2562047788:00:54.775808

This bogus value returned on REL_15_STABLE .. master, but on e39f99046~1
I get:
ERROR: interval out of range

without ubsan. Though with the sanitizer I see the same complaint (and it
can be seen on previous branches including REL_12_STABLE):
timestamp.c:3318:18: runtime error: 1.2e+10 is outside the range of
representable values of type 'int'

#2Laurenz Albe
laurenz.albe@cybertec.at
In reply to: PG Bug reporting form (#1)
Re: BUG #18200: Undefined behaviour in interval_div

On Wed, 2023-11-15 at 13:00 +0000, PG Bug reporting form wrote:

The following query:
SELECT interval '12000000 months' / 0.001;

triggers an ubsan-detected error:
timestamp.c:3408:18: runtime error: 1.2e+10 is outside the range of
representable values of type 'int'

Without ubsan the result is:
-178956970 years -8 mons -2562047788:00:54.775808

How about the attached fix?

Yours,
Laurenz Albe

Attachments:

0001-Check-for-overflow-in-interval-division.patchtext/x-patch; charset=UTF-8; name=0001-Check-for-overflow-in-interval-division.patchDownload+9-1
#3Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Laurenz Albe (#2)
Re: BUG #18200: Undefined behaviour in interval_div

On Wed, 15 Nov 2023 at 17:23, Laurenz Albe <laurenz.albe@cybertec.at> wrote:

How about the attached fix?

I don't think that's sufficient -- interval_div() should have all the
same overflow protections that interval_mul() has, since they're
basically doing the same thing.

However, I don't think that's sufficient either -- looking at
interval_mul(), there is nothing to prevent integer overflow when
cascading down, so additional checks are needed there too (in both
functions).

Regards,
Dean

#4Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Dean Rasheed (#3)
Re: BUG #18200: Undefined behaviour in interval_div

On Wed, 15 Nov 2023 at 18:14, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

looking at
interval_mul(), there is nothing to prevent integer overflow when
cascading down, so additional checks are needed there too (in both
functions).

Here's a patch doing that. I'm inclined to back-patch this, because
this seems like something that's quite easily triggered, and returning
bogus results is not good.

Regards,
Dean

Attachments:

detect-interval-mul-div-overflow.patchtext/x-patch; charset=US-ASCII; name=detect-interval-mul-div-overflow.patchDownload+80-44
#5Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Dean Rasheed (#4)
Re: BUG #18200: Undefined behaviour in interval_div

On Wed, 15 Nov 2023 at 22:41, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

On Wed, 15 Nov 2023 at 18:14, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

looking at
interval_mul(), there is nothing to prevent integer overflow when
cascading down, so additional checks are needed there too (in both
functions).

Here's a patch doing that. I'm inclined to back-patch this

Pushed and back-patched.

Regards,
Dean