Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 17db543

Browse files
tglsfdctglsfdc
committed
Fix mis-rounding and overflow hazards in date_bin().
In the case where the target timestamp is before the origin timestamp and their difference is already an exact multiple of the stride, the code incorrectly subtracted the stride anyway. Also detect several integer-overflow cases that previously produced bogus results. (The submitted patch tried to avoid overflow, but I'm not convinced it's right, and problematic cases are so far out of the plausibly-useful range that they don't seem worth sweating over. Let's just use overflow-detecting arithmetic and throw errors.) timestamp_bin() and timestamptz_bin() are basically identical and so had identical bugs. Fix both. Report and patch by Moaaz Assali, adjusted some by me. Back-patch to v14 where date_bin() was introduced. Discussion: https://postgr.es/m/CALkF+nvtuas-2kydG-WfofbRSJpyODAJWun==W-yO5j2R4meqA@mail.gmail.com
1 parent c4bd6ff commit 17db543

File tree

5 files changed

+97
-19
lines changed

5 files changed

+97
-19
lines changed

‎src/backend/utils/adt/timestamp.c

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3989,8 +3989,9 @@ timestamp_bin(PG_FUNCTION_ARGS)
39893989
Timestamp timestamp = PG_GETARG_TIMESTAMP(1);
39903990
Timestamp origin = PG_GETARG_TIMESTAMP(2);
39913991
Timestamp result,
3992-
tm_diff,
39933992
stride_usecs,
3993+
tm_diff,
3994+
tm_modulo,
39943995
tm_delta;
39953996

39963997
if (TIMESTAMP_NOT_FINITE(timestamp))
@@ -4006,24 +4007,40 @@ timestamp_bin(PG_FUNCTION_ARGS)
40064007
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
40074008
errmsg("timestamps cannot be binned into intervals containing months or years")));
40084009

4009-
stride_usecs = stride->day * USECS_PER_DAY + stride->time;
4010+
if (unlikely(pg_mul_s64_overflow(stride->day, USECS_PER_DAY, &stride_usecs)) ||
4011+
unlikely(pg_add_s64_overflow(stride_usecs, stride->time, &stride_usecs)))
4012+
ereport(ERROR,
4013+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4014+
errmsg("interval out of range")));
40104015

40114016
if (stride_usecs <= 0)
40124017
ereport(ERROR,
40134018
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
40144019
errmsg("stride must be greater than zero")));
40154020

4016-
tm_diff = timestamp - origin;
4017-
tm_delta = tm_diff - tm_diff % stride_usecs;
4021+
if (unlikely(pg_sub_s64_overflow(timestamp, origin, &tm_diff)))
4022+
ereport(ERROR,
4023+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4024+
errmsg("interval out of range")));
4025+
4026+
/* These calculations cannot overflow */
4027+
tm_modulo = tm_diff % stride_usecs;
4028+
tm_delta = tm_diff - tm_modulo;
4029+
result = origin + tm_delta;
40184030

40194031
/*
4020-
* Make sure the returned timestamp is at the start of the bin, even if
4021-
* the origin is in the future.
4032+
* We want to round towards -infinity, not 0, when tm_diff is negative and
4033+
* not a multiple of stride_usecs. This adjustment *can* cause overflow,
4034+
* since the result might now be out of the range origin .. timestamp.
40224035
*/
4023-
if (origin > timestamp && stride_usecs > 1)
4024-
tm_delta -= stride_usecs;
4025-
4026-
result = origin + tm_delta;
4036+
if (tm_modulo < 0)
4037+
{
4038+
if (unlikely(pg_sub_s64_overflow(result, stride_usecs, &result)) ||
4039+
!IS_VALID_TIMESTAMP(result))
4040+
ereport(ERROR,
4041+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4042+
errmsg("timestamp out of range")));
4043+
}
40274044

40284045
PG_RETURN_TIMESTAMP(result);
40294046
}
@@ -4174,6 +4191,7 @@ timestamptz_bin(PG_FUNCTION_ARGS)
41744191
TimestampTz result,
41754192
stride_usecs,
41764193
tm_diff,
4194+
tm_modulo,
41774195
tm_delta;
41784196

41794197
if (TIMESTAMP_NOT_FINITE(timestamp))
@@ -4189,24 +4207,40 @@ timestamptz_bin(PG_FUNCTION_ARGS)
41894207
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
41904208
errmsg("timestamps cannot be binned into intervals containing months or years")));
41914209

4192-
stride_usecs = stride->day * USECS_PER_DAY + stride->time;
4210+
if (unlikely(pg_mul_s64_overflow(stride->day, USECS_PER_DAY, &stride_usecs)) ||
4211+
unlikely(pg_add_s64_overflow(stride_usecs, stride->time, &stride_usecs)))
4212+
ereport(ERROR,
4213+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4214+
errmsg("interval out of range")));
41934215

41944216
if (stride_usecs <= 0)
41954217
ereport(ERROR,
41964218
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
41974219
errmsg("stride must be greater than zero")));
41984220

4199-
tm_diff = timestamp - origin;
4200-
tm_delta = tm_diff - tm_diff % stride_usecs;
4221+
if (unlikely(pg_sub_s64_overflow(timestamp, origin, &tm_diff)))
4222+
ereport(ERROR,
4223+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4224+
errmsg("interval out of range")));
4225+
4226+
/* These calculations cannot overflow */
4227+
tm_modulo = tm_diff % stride_usecs;
4228+
tm_delta = tm_diff - tm_modulo;
4229+
result = origin + tm_delta;
42014230

42024231
/*
4203-
* Make sure the returned timestamp is at the start of the bin, even if
4204-
* the origin is in the future.
4232+
* We want to round towards -infinity, not 0, when tm_diff is negative and
4233+
* not a multiple of stride_usecs. This adjustment *can* cause overflow,
4234+
* since the result might now be out of the range origin .. timestamp.
42054235
*/
4206-
if (origin > timestamp && stride_usecs > 1)
4207-
tm_delta -= stride_usecs;
4208-
4209-
result = origin + tm_delta;
4236+
if (tm_modulo < 0)
4237+
{
4238+
if (unlikely(pg_sub_s64_overflow(result, stride_usecs, &result)) ||
4239+
!IS_VALID_TIMESTAMP(result))
4240+
ereport(ERROR,
4241+
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
4242+
errmsg("timestamp out of range")));
4243+
}
42104244

42114245
PG_RETURN_TIMESTAMPTZ(result);
42124246
}

‎src/test/regress/expected/timestamp.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,13 @@ SELECT date_bin('5 min'::interval, timestamp '2020-02-01 01:01:01', timestamp '2
736736
Sat Feb 01 00:57:30 2020
737737
(1 row)
738738

739+
-- test roundoff edge case when source < origin
740+
SELECT date_bin('30 minutes'::interval, timestamp '2024-02-01 15:00:00', timestamp '2024-02-01 17:00:00');
741+
date_bin
742+
--------------------------
743+
Thu Feb 01 15:00:00 2024
744+
(1 row)
745+
739746
-- disallow intervals with months or years
740747
SELECT date_bin('5 months'::interval, timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
741748
ERROR: timestamps cannot be binned into intervals containing months or years
@@ -747,6 +754,13 @@ ERROR: stride must be greater than zero
747754
-- disallow negative intervals
748755
SELECT date_bin('-2 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp '1970-01-01 00:00:00');
749756
ERROR: stride must be greater than zero
757+
-- test overflow cases
758+
select date_bin('15 minutes'::interval, timestamp '294276-12-30', timestamp '4000-12-20 BC');
759+
ERROR: interval out of range
760+
select date_bin('200000000 days'::interval, '2024-02-01'::timestamp, '2024-01-01'::timestamp);
761+
ERROR: interval out of range
762+
select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamp, '4000-01-01 BC'::timestamp);
763+
ERROR: timestamp out of range
750764
-- Test casting within a BETWEEN qualifier
751765
SELECT d1 - timestamp without time zone '1997-01-02' AS diff
752766
FROM TIMESTAMP_TBL

‎src/test/regress/expected/timestamptz.out

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,13 @@ SELECT date_bin('5 min'::interval, timestamptz '2020-02-01 01:01:01+00', timesta
780780
Fri Jan 31 16:57:30 2020 PST
781781
(1 row)
782782

783+
-- test roundoff edge case when source < origin
784+
SELECT date_bin('30 minutes'::interval, timestamptz '2024-02-01 15:00:00', timestamptz '2024-02-01 17:00:00');
785+
date_bin
786+
------------------------------
787+
Thu Feb 01 15:00:00 2024 PST
788+
(1 row)
789+
783790
-- disallow intervals with months or years
784791
SELECT date_bin('5 months'::interval, timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
785792
ERROR: timestamps cannot be binned into intervals containing months or years
@@ -791,6 +798,13 @@ ERROR: stride must be greater than zero
791798
-- disallow negative intervals
792799
SELECT date_bin('-2 days'::interval, timestamp with time zone '1970-01-01 01:00:00+00' , timestamp with time zone '1970-01-01 00:00:00+00');
793800
ERROR: stride must be greater than zero
801+
-- test overflow cases
802+
select date_bin('15 minutes'::interval, timestamptz '294276-12-30', timestamptz '4000-12-20 BC');
803+
ERROR: interval out of range
804+
select date_bin('200000000 days'::interval, '2024-02-01'::timestamptz, '2024-01-01'::timestamptz);
805+
ERROR: interval out of range
806+
select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamptz, '4000-01-01 BC'::timestamptz);
807+
ERROR: timestamp out of range
794808
-- Test casting within a BETWEEN qualifier
795809
SELECT d1 - timestamp with time zone '1997-01-02' AS diff
796810
FROM TIMESTAMPTZ_TBL

‎src/test/regress/sql/timestamp.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,9 @@ FROM (
268268
-- shift bins using the origin parameter:
269269
SELECT date_bin('5 min'::interval, timestamp '2020-02-01 01:01:01', timestamp '2020-02-01 00:02:30');
270270

271+
-- test roundoff edge case when source < origin
272+
SELECT date_bin('30 minutes'::interval, timestamp '2024-02-01 15:00:00', timestamp '2024-02-01 17:00:00');
273+
271274
-- disallow intervals with months or years
272275
SELECT date_bin('5 months'::interval, timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
273276
SELECT date_bin('5 years'::interval, timestamp '2020-02-01 01:01:01', timestamp '2001-01-01');
@@ -278,6 +281,11 @@ SELECT date_bin('0 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp
278281
-- disallow negative intervals
279282
SELECT date_bin('-2 days'::interval, timestamp '1970-01-01 01:00:00' , timestamp '1970-01-01 00:00:00');
280283

284+
-- test overflow cases
285+
select date_bin('15 minutes'::interval, timestamp '294276-12-30', timestamp '4000-12-20 BC');
286+
select date_bin('200000000 days'::interval, '2024-02-01'::timestamp, '2024-01-01'::timestamp);
287+
select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamp, '4000-01-01 BC'::timestamp);
288+
281289
-- Test casting within a BETWEEN qualifier
282290
SELECT d1 - timestamp without time zone '1997-01-02' AS diff
283291
FROM TIMESTAMP_TBL

‎src/test/regress/sql/timestamptz.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,9 @@ FROM (
243243
-- shift bins using the origin parameter:
244244
SELECT date_bin('5 min'::interval, timestamptz '2020-02-01 01:01:01+00', timestamptz '2020-02-01 00:02:30+00');
245245

246+
-- test roundoff edge case when source < origin
247+
SELECT date_bin('30 minutes'::interval, timestamptz '2024-02-01 15:00:00', timestamptz '2024-02-01 17:00:00');
248+
246249
-- disallow intervals with months or years
247250
SELECT date_bin('5 months'::interval, timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
248251
SELECT date_bin('5 years'::interval, timestamp with time zone '2020-02-01 01:01:01+00', timestamp with time zone '2001-01-01+00');
@@ -253,6 +256,11 @@ SELECT date_bin('0 days'::interval, timestamp with time zone '1970-01-01 01:00:0
253256
-- disallow negative intervals
254257
SELECT date_bin('-2 days'::interval, timestamp with time zone '1970-01-01 01:00:00+00' , timestamp with time zone '1970-01-01 00:00:00+00');
255258

259+
-- test overflow cases
260+
select date_bin('15 minutes'::interval, timestamptz '294276-12-30', timestamptz '4000-12-20 BC');
261+
select date_bin('200000000 days'::interval, '2024-02-01'::timestamptz, '2024-01-01'::timestamptz);
262+
select date_bin('365000 days'::interval, '4400-01-01 BC'::timestamptz, '4000-01-01 BC'::timestamptz);
263+
256264
-- Test casting within a BETWEEN qualifier
257265
SELECT d1 - timestamp with time zone '1997-01-02' AS diff
258266
FROM TIMESTAMPTZ_TBL

0 commit comments

Comments
 (0)