pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18200: Undefined behaviour in interval_div
5+ messages / 3 participants
[nested] [flat]
* BUG #18200: Undefined behaviour in interval_div
@ 2023-11-15 13:00 PG Bug reporting form <noreply@postgresql.org>
2023-11-15 17:23 ` Re: BUG #18200: Undefined behaviour in interval_div Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 5+ messages in thread
From: PG Bug reporting form @ 2023-11-15 13:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
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'
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18200: Undefined behaviour in interval_div
2023-11-15 13:00 BUG #18200: Undefined behaviour in interval_div PG Bug reporting form <noreply@postgresql.org>
@ 2023-11-15 17:23 ` Laurenz Albe <laurenz.albe@cybertec.at>
2023-11-15 18:14 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Laurenz Albe @ 2023-11-15 17:23 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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:
[text/x-patch] 0001-Check-for-overflow-in-interval-division.patch (1.2K, ../../c8374b1627add66a9593de562be16142cd690095.camel@cybertec.at/2-0001-Check-for-overflow-in-interval-division.patch)
download | inline diff:
From 2748c01a3e343001d3668554fdb7ee894fefbdf1 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Wed, 15 Nov 2023 18:21:42 +0100
Subject: [PATCH] Check for overflow in interval division
Dividing a large interval by a small floating point number
can lead to integer overflow, which results in bogus values.
Report by Alexander Lakhin.
Discussion: https://postgr.es/m/18200-5ea288c7b2d504b1@postgresql.org
---
src/backend/utils/adt/timestamp.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 647b97aca6..b21efeb897 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3422,6 +3422,15 @@ interval_div(PG_FUNCTION_ARGS)
(errcode(ERRCODE_DIVISION_BY_ZERO),
errmsg("division by zero")));
+ /* check for overflow */
+ if (span->month / factor > PG_INT32_MAX ||
+ span->month / factor < PG_INT32_MIN ||
+ span->day / factor > PG_INT32_MAX ||
+ span->day / factor < PG_INT32_MIN)
+ ereport(ERROR,
+ errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range"));
+
result->month = (int32) (span->month / factor);
result->day = (int32) (span->day / factor);
--
2.41.0
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18200: Undefined behaviour in interval_div
2023-11-15 13:00 BUG #18200: Undefined behaviour in interval_div PG Bug reporting form <noreply@postgresql.org>
2023-11-15 17:23 ` Re: BUG #18200: Undefined behaviour in interval_div Laurenz Albe <laurenz.albe@cybertec.at>
@ 2023-11-15 18:14 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-11-15 22:41 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Dean Rasheed @ 2023-11-15 18:14 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18200: Undefined behaviour in interval_div
2023-11-15 13:00 BUG #18200: Undefined behaviour in interval_div PG Bug reporting form <noreply@postgresql.org>
2023-11-15 17:23 ` Re: BUG #18200: Undefined behaviour in interval_div Laurenz Albe <laurenz.albe@cybertec.at>
2023-11-15 18:14 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2023-11-15 22:41 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-11-18 15:16 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Dean Rasheed @ 2023-11-15 22:41 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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:
[text/x-patch] detect-interval-mul-div-overflow.patch (7.6K, ../../CAEZATCWar0wEJw_eQudbnpZsyC5VCcJ=hngaUD0ZEWWrm2LGbA@mail.gmail.com/2-detect-interval-mul-div-overflow.patch)
download | inline diff:
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
new file mode 100644
index 45abb79..eebc591
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3548,17 +3548,14 @@ interval_mul(PG_FUNCTION_ARGS)
* interval type has nothing equivalent to NaN.
*/
if (isnan(factor))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
if (INTERVAL_NOT_FINITE(span))
{
if (factor == 0.0)
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
- else if (factor < 0.0)
+ goto out_of_range;
+
+ if (factor < 0.0)
interval_um_internal(span, result);
else
memcpy(result, span, sizeof(Interval));
@@ -3570,10 +3567,9 @@ interval_mul(PG_FUNCTION_ARGS)
int isign = interval_sign(span);
if (isign == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
- else if (factor * isign < 0)
+ goto out_of_range;
+
+ if (factor * isign < 0)
INTERVAL_NOBEGIN(result);
else
INTERVAL_NOEND(result);
@@ -3582,19 +3578,13 @@ interval_mul(PG_FUNCTION_ARGS)
}
result_double = span->month * factor;
- if (isnan(result_double) ||
- result_double > INT_MAX || result_double < INT_MIN)
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ if (isnan(result_double) || !FLOAT8_FITS_IN_INT32(result_double))
+ goto out_of_range;
result->month = (int32) result_double;
result_double = span->day * factor;
- if (isnan(result_double) ||
- result_double > INT_MAX || result_double < INT_MIN)
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ if (isnan(result_double) || !FLOAT8_FITS_IN_INT32(result_double))
+ goto out_of_range;
result->day = (int32) result_double;
/*
@@ -3628,25 +3618,33 @@ interval_mul(PG_FUNCTION_ARGS)
*/
if (fabs(sec_remainder) >= SECS_PER_DAY)
{
- result->day += (int) (sec_remainder / SECS_PER_DAY);
+ if (pg_add_s32_overflow(result->day,
+ (int) (sec_remainder / SECS_PER_DAY),
+ &result->day))
+ goto out_of_range;
sec_remainder -= (int) (sec_remainder / SECS_PER_DAY) * SECS_PER_DAY;
}
/* cascade units down */
- result->day += (int32) month_remainder_days;
+ if (pg_add_s32_overflow(result->day, (int32) month_remainder_days,
+ &result->day))
+ goto out_of_range;
result_double = rint(span->time * factor + sec_remainder * USECS_PER_SEC);
if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
result->time = (int64) result_double;
if (INTERVAL_NOT_FINITE(result))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
PG_RETURN_INTERVAL_P(result);
+
+out_of_range:
+ ereport(ERROR,
+ errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range"));
+
+ PG_RETURN_NULL(); /* keep compiler quiet */
}
Datum
@@ -3665,7 +3663,8 @@ interval_div(PG_FUNCTION_ARGS)
Interval *span = PG_GETARG_INTERVAL_P(0);
float8 factor = PG_GETARG_FLOAT8(1);
double month_remainder_days,
- sec_remainder;
+ sec_remainder,
+ result_double;
int32 orig_month = span->month,
orig_day = span->day;
Interval *result;
@@ -3685,16 +3684,12 @@ interval_div(PG_FUNCTION_ARGS)
* by the regular division code, causing all fields to be set to zero.
*/
if (isnan(factor))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
if (INTERVAL_NOT_FINITE(span))
{
if (isinf(factor))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
if (factor < 0.0)
interval_um_internal(span, result);
@@ -3704,8 +3699,15 @@ interval_div(PG_FUNCTION_ARGS)
PG_RETURN_INTERVAL_P(result);
}
- result->month = (int32) (span->month / factor);
- result->day = (int32) (span->day / factor);
+ result_double = span->month / factor;
+ if (isnan(result_double) || !FLOAT8_FITS_IN_INT32(result_double))
+ goto out_of_range;
+ result->month = (int32) result_double;
+
+ result_double = span->day / factor;
+ if (isnan(result_double) || !FLOAT8_FITS_IN_INT32(result_double))
+ goto out_of_range;
+ result->day = (int32) result_double;
/*
* Fractional months full days into days. See comment in interval_mul().
@@ -3717,20 +3719,33 @@ interval_div(PG_FUNCTION_ARGS)
sec_remainder = TSROUND(sec_remainder);
if (fabs(sec_remainder) >= SECS_PER_DAY)
{
- result->day += (int) (sec_remainder / SECS_PER_DAY);
+ if (pg_add_s32_overflow(result->day,
+ (int) (sec_remainder / SECS_PER_DAY),
+ &result->day))
+ goto out_of_range;
sec_remainder -= (int) (sec_remainder / SECS_PER_DAY) * SECS_PER_DAY;
}
/* cascade units down */
- result->day += (int32) month_remainder_days;
- result->time = rint(span->time / factor + sec_remainder * USECS_PER_SEC);
+ if (pg_add_s32_overflow(result->day, (int32) month_remainder_days,
+ &result->day))
+ goto out_of_range;
+ result_double = rint(span->time / factor + sec_remainder * USECS_PER_SEC);
+ if (isnan(result_double) || !FLOAT8_FITS_IN_INT64(result_double))
+ goto out_of_range;
+ result->time = (int64) result_double;
if (INTERVAL_NOT_FINITE(result))
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("interval out of range")));
+ goto out_of_range;
PG_RETURN_INTERVAL_P(result);
+
+out_of_range:
+ ereport(ERROR,
+ errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("interval out of range"));
+
+ PG_RETURN_NULL(); /* keep compiler quiet */
}
diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
new file mode 100644
index a481781..b79b6fc
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -488,6 +488,19 @@ SELECT * FROM INTERVAL_TBL;
-infinity
(12 rows)
+-- multiplication and division overflow test cases
+SELECT '3000000 months'::interval * 1000;
+ERROR: interval out of range
+SELECT '3000000 months'::interval / 0.001;
+ERROR: interval out of range
+SELECT '3000000 days'::interval * 1000;
+ERROR: interval out of range
+SELECT '3000000 days'::interval / 0.001;
+ERROR: interval out of range
+SELECT '1 month 2146410 days'::interval * 1000.5002;
+ERROR: interval out of range
+SELECT '4611686018427387904 usec'::interval / 0.1;
+ERROR: interval out of range
-- test avg(interval), which is somewhat fragile since people have been
-- known to change the allowed input syntax for type interval without
-- updating pg_aggregate.agginitval
diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
new file mode 100644
index caad291..5566ad0
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -154,6 +154,14 @@ SET IntervalStyle to postgres_verbose;
SELECT * FROM INTERVAL_TBL;
+-- multiplication and division overflow test cases
+SELECT '3000000 months'::interval * 1000;
+SELECT '3000000 months'::interval / 0.001;
+SELECT '3000000 days'::interval * 1000;
+SELECT '3000000 days'::interval / 0.001;
+SELECT '1 month 2146410 days'::interval * 1000.5002;
+SELECT '4611686018427387904 usec'::interval / 0.1;
+
-- test avg(interval), which is somewhat fragile since people have been
-- known to change the allowed input syntax for type interval without
-- updating pg_aggregate.agginitval
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: BUG #18200: Undefined behaviour in interval_div
2023-11-15 13:00 BUG #18200: Undefined behaviour in interval_div PG Bug reporting form <noreply@postgresql.org>
2023-11-15 17:23 ` Re: BUG #18200: Undefined behaviour in interval_div Laurenz Albe <laurenz.albe@cybertec.at>
2023-11-15 18:14 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
2023-11-15 22:41 ` Re: BUG #18200: Undefined behaviour in interval_div Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2023-11-18 15:16 ` Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 0 replies; 5+ messages in thread
From: Dean Rasheed @ 2023-11-18 15:16 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-11-18 15:16 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-11-15 13:00 BUG #18200: Undefined behaviour in interval_div PG Bug reporting form <noreply@postgresql.org>
2023-11-15 17:23 ` Laurenz Albe <laurenz.albe@cybertec.at>
2023-11-15 18:14 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-11-15 22:41 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2023-11-18 15:16 ` Dean Rasheed <dean.a.rasheed@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox