On Fri, Dec 06, 2024 at 10:22:44AM -0600, Nathan Bossart wrote: > On Thu, Dec 05, 2024 at 09:58:29PM -0600, Nathan Bossart wrote: >> Thanks for reviewing. In v28, I fixed a silly mistake revealed by cfbot's >> Windows run. I also went ahead and tried to fix most of the issues >> reported in a nearby thread [0]. The only one I haven't tracked down yet >> is the "ISO week" one (case 7). > > The overflow hazard for the ISO week case seems to be in isoweek2j(), which > would require quite a bit of restructuring to work with the soft-error > handling from its callers. I'm not feeling particularly inspired to do > that, so here's a v29 with a comment added above that function.
I seem to have a knack for picking patches that take an entire afternoon to back-patch... Here's what I have staged for commit early next week. -- nathan
>From c97730ef52d99832bf74971d40dba915a93c1017 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 9 ++- src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++-- src/backend/utils/adt/timestamp.c | 4 + src/include/common/int.h | 48 ++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 ++ 8 files changed, 183 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 8130f3e8ac..da61ac0e86 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 2bcc185708..a9daa5c591 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -77,6 +77,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "common/unicode_case.h" #include "common/unicode_category.h" #include "mb/pg_wchar.h" @@ -3826,7 +3827,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, ereturn(escontext,, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\""))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + ereturn(escontext,, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range"))); + if (!from_char_set_int(&out->year, years, n, escontext)) return; out->yysz = 4; @@ -4785,10 +4793,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } } else { @@ -4814,11 +4847,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } fmask |= DTK_M(YEAR); } @@ -4843,11 +4896,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4912,7 +4989,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 57fcfefdaf..18d7d8a108 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -5104,6 +5104,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index 3b1590d676..6b50aa67b9 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -117,6 +117,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + static inline uint16 pg_abs_s16(int16 a) { @@ -185,6 +201,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + static inline uint32 pg_abs_s32(int32 a) { @@ -300,6 +332,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + static inline uint64 pg_abs_s64(int64 a) { diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index c9cec70c38..dcab9e76f4 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index 6d7dd5c988..b87b88f038 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- to_char's TZ format code produces zone abbrev if known SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ'); to_char diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 1c58ff6966..805aec706c 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -371,5 +371,6 @@ select make_date(0, 7, 15); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index 0fe3c783e6..808083a6d8 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- to_char's TZ format code produces zone abbrev if known SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ'); -- 2.39.5 (Apple Git-154)
>From 577793deb242e373ac6e85fbfa6ce696b4c6c792 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 10 +++- src/backend/utils/adt/formatting.c | 80 +++++++++++++++++++++++--- src/backend/utils/adt/timestamp.c | 4 ++ src/include/common/int.h | 51 ++++++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 +++ 8 files changed, 163 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index baa069125e..64f28ef6ee 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -23,6 +23,7 @@ #include "access/xact.h" #include "common/hashfn.h" +#include "common/int.h" #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/supportnodes.h" @@ -247,8 +248,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index e4ffecb9d8..f967c37038 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -85,6 +85,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "mb/pg_wchar.h" #include "parser/scansup.h" #include "utils/builtins.h" @@ -3683,7 +3684,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, RETURN_ERROR(ereport(ERROR, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\"")))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + RETURN_ERROR(ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range")))); + from_char_set_int(&out->year, years, n, have_error); CHECK_ERROR; out->yysz = 4; @@ -4598,10 +4606,29 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } } else { @@ -4627,11 +4654,25 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } fmask |= DTK_M(YEAR); } @@ -4656,11 +4697,27 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4724,7 +4781,14 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 637f9b8ed4..1f15ca494d 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -4331,6 +4331,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index 4c862651f5..639e892072 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,9 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a + * into *result. The content of *result is implementation defined in case of + * overflow. *--------- */ @@ -97,6 +100,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT32 */ @@ -154,6 +173,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT64 */ @@ -258,6 +293,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index 7d753654f8..1e3eed11bb 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1470,6 +1470,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index ae447b8a3a..e83cb30e69 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3269,6 +3269,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3309,6 +3317,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 4c5b94a14a..2e41674ecc 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -343,5 +343,6 @@ select make_time(8, 20, 0.0); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index 4ab5c1dfc8..1bd0d22a48 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -542,6 +542,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -552,6 +556,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- 2.39.5 (Apple Git-154)
>From f8d2b13551698346bf1545ef7a0cde18c0701efe Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 10 +++- src/backend/utils/adt/formatting.c | 80 +++++++++++++++++++++++--- src/backend/utils/adt/timestamp.c | 4 ++ src/include/common/int.h | 51 ++++++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 +++ 8 files changed, 163 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index eac296e318..a6655f2a2a 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -23,6 +23,7 @@ #include "access/xact.h" #include "common/hashfn.h" +#include "common/int.h" #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/supportnodes.h" @@ -248,8 +249,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 5bd4d1967b..bf4c37814a 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -85,6 +85,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "mb/pg_wchar.h" #include "parser/scansup.h" #include "utils/builtins.h" @@ -3682,7 +3683,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, RETURN_ERROR(ereport(ERROR, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\"")))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + RETURN_ERROR(ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range")))); + from_char_set_int(&out->year, years, n, have_error); CHECK_ERROR; out->yysz = 4; @@ -4597,10 +4605,29 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } } else { @@ -4626,11 +4653,25 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } fmask |= DTK_M(YEAR); } @@ -4655,11 +4696,27 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4723,7 +4780,14 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 56bdb6b8d8..4e06d64cf6 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -4468,6 +4468,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index 5ba9dd88e7..86896d1a4f 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,9 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a + * into *result. The content of *result is implementation defined in case of + * overflow. *--------- */ @@ -97,6 +100,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT32 */ @@ -154,6 +173,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT64 */ @@ -258,6 +293,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index 958b5f15e4..3d3ef4c3d4 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1491,6 +1491,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index e4b57f6215..d8baed2fbd 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3269,6 +3269,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3309,6 +3317,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 8f7435b767..a7a9f67b6f 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -362,5 +362,6 @@ select make_date(0, 7, 15); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index 13f86ce8fc..5ff60c750c 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -542,6 +542,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -552,6 +556,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- 2.39.5 (Apple Git-154)
>From 3fb3437afc087a411c2cd179d625cf273ec0d287 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 10 +++- src/backend/utils/adt/formatting.c | 80 +++++++++++++++++++++++--- src/backend/utils/adt/timestamp.c | 4 ++ src/include/common/int.h | 51 ++++++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 +++ 8 files changed, 163 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 44b75fe1b4..d3d35a68d2 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -24,6 +24,7 @@ #include "access/xact.h" #include "catalog/pg_type.h" #include "common/hashfn.h" +#include "common/int.h" #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/supportnodes.h" @@ -249,8 +250,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 7b1f080f80..2aeb253a57 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -85,6 +85,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "mb/pg_wchar.h" #include "parser/scansup.h" #include "utils/builtins.h" @@ -3713,7 +3714,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, RETURN_ERROR(ereport(ERROR, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\"")))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + RETURN_ERROR(ereport(ERROR, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range")))); + from_char_set_int(&out->year, years, n, have_error); CHECK_ERROR; out->yysz = 4; @@ -4644,10 +4652,29 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } } else { @@ -4673,11 +4700,25 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, + text_to_cstring(date_txt), + "timestamp")); + } fmask |= DTK_M(YEAR); } @@ -4702,11 +4743,27 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4770,7 +4827,14 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + RETURN_ERROR(DateTimeParseError(DTERR_FIELD_OVERFLOW, date_str, "timestamp")); + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index 1544d54460..01193406cb 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -4511,6 +4511,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index e2617fbc5d..c4a360328e 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,9 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a + * into *result. The content of *result is implementation defined in case of + * overflow. *--------- */ @@ -97,6 +100,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT32 */ @@ -154,6 +173,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT64 */ @@ -258,6 +293,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index 341123978c..6532b1703f 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1491,6 +1491,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index e41a71a672..a216d5f066 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3344,6 +3344,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3384,6 +3392,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 8f7435b767..a7a9f67b6f 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -362,5 +362,6 @@ select make_date(0, 7, 15); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index d3a68264d7..c0f8ae72b4 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -572,6 +572,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -582,6 +586,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- 2.39.5 (Apple Git-154)
>From 118fb03f3c6c3aa924b54a1c58d005ed27fe688b Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 10 ++- src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++-- src/backend/utils/adt/timestamp.c | 4 + src/include/common/int.h | 51 ++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 ++ 8 files changed, 187 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 5420de8342..fa721c91e6 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -24,6 +24,7 @@ #include "access/xact.h" #include "catalog/pg_type.h" #include "common/hashfn.h" +#include "common/int.h" #include "libpq/pqformat.h" #include "miscadmin.h" #include "nodes/supportnodes.h" @@ -255,8 +256,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index e6246dc44b..f967ff55ff 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -77,6 +77,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "mb/pg_wchar.h" #include "nodes/miscnodes.h" #include "parser/scansup.h" @@ -3663,7 +3664,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, ereturn(escontext,, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\""))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + ereturn(escontext,, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range"))); + if (!from_char_set_int(&out->year, years, n, escontext)) return; out->yysz = 4; @@ -4599,10 +4607,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } } else { @@ -4628,11 +4661,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } fmask |= DTK_M(YEAR); } @@ -4657,11 +4710,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4726,7 +4803,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index fd92287948..c3b7b7979c 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -4546,6 +4546,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index 487124473d..db944c1a5d 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,9 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a + * into *result. The content of *result is implementation defined in case of + * overflow. *--------- */ @@ -97,6 +100,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT32 */ @@ -154,6 +173,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT64 */ @@ -258,6 +293,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index 20374c5230..0e7f7534cc 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index 0681f84d5f..45a424027a 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3608,6 +3608,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3648,6 +3656,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 1c58ff6966..805aec706c 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -371,5 +371,6 @@ select make_date(0, 7, 15); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index fdd70a0767..613c924393 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -635,6 +635,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -645,6 +649,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- -- Check behavior with SQL-style fixed-GMT-offset time zone (cf bug #8572) -- 2.39.5 (Apple Git-154)
>From f1f55816568935cd44a2f46bb068ce365899470a Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Fri, 6 Dec 2024 14:07:11 -0600 Subject: [PATCH v30 1/1] Fix various overflow hazards in date and timestamp functions. This commit makes use of the overflow-aware routines in int.h to fix a variety of reported overflow bugs in the date and timestamp code. It seems unlikely that this fixes all such bugs in this area, but since the problems seem limited to cases that are far beyond any realistic use-case, I'm not going to worry too much. Note that for one bug, I've chosen to simply add a comment about the overflow hazard because fixing it would require quite a bit of code restructuring that doesn't seem worth the risk. Reported-by: Alexander Lakhin Author: Matthew Kim, Nathan Bossart Reviewed-by: Joseph Koshakow, Jian He Discussion: https://postgr.es/m/31ad2cd1-db94-bdb3-f91a-65ffdb4bef95%40gmail.com Discussion: https://postgr.es/m/18585-db646741dd649abd%40postgresql.org Backpatch-through: 13 --- src/backend/utils/adt/date.c | 9 ++- src/backend/utils/adt/formatting.c | 104 +++++++++++++++++++++++-- src/backend/utils/adt/timestamp.c | 4 + src/include/common/int.h | 51 ++++++++++++ src/test/regress/expected/date.out | 2 + src/test/regress/expected/horology.out | 16 ++++ src/test/regress/sql/date.sql | 1 + src/test/regress/sql/horology.sql | 8 ++ 8 files changed, 186 insertions(+), 9 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 9c854e0e5c..837b8cccdf 100644 --- a/src/backend/utils/adt/date.c +++ b/src/backend/utils/adt/date.c @@ -256,8 +256,15 @@ make_date(PG_FUNCTION_ARGS) /* Handle negative years as BC */ if (tm.tm_year < 0) { + int year = tm.tm_year; + bc = true; - tm.tm_year = -tm.tm_year; + if (pg_neg_s32_overflow(year, &year)) + ereport(ERROR, + (errcode(ERRCODE_DATETIME_FIELD_OVERFLOW), + errmsg("date field value out of range: %d-%02d-%02d", + tm.tm_year, tm.tm_mon, tm.tm_mday))); + tm.tm_year = year; } dterr = ValidateDate(DTK_DATE_M, false, false, bc, &tm); diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 8736ada4be..e96a348835 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -77,6 +77,7 @@ #include "catalog/pg_collation.h" #include "catalog/pg_type.h" +#include "common/int.h" #include "common/unicode_case.h" #include "common/unicode_category.h" #include "mb/pg_wchar.h" @@ -3851,7 +3852,14 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out, ereturn(escontext,, (errcode(ERRCODE_INVALID_DATETIME_FORMAT), errmsg("invalid input string for \"Y,YYY\""))); - years += (millennia * 1000); + + /* years += (millennia * 1000); */ + if (pg_mul_s32_overflow(millennia, 1000, &millennia) || + pg_add_s32_overflow(years, millennia, &years)) + ereturn(escontext,, + (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE), + errmsg("value for \"Y,YYY\" in source string is out of range"))); + if (!from_char_set_int(&out->year, years, n, escontext)) return; out->yysz = 4; @@ -4810,10 +4818,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, tm->tm_year = tmfc.year % 100; if (tm->tm_year) { + int tmp; + if (tmfc.cc >= 0) - tm->tm_year += (tmfc.cc - 1) * 100; + { + /* tm->tm_year += (tmfc.cc - 1) * 100; */ + tmp = tmfc.cc - 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_add_s32_overflow(tm->tm_year, tmp, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else - tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; + { + /* tm->tm_year = (tmfc.cc + 1) * 100 - tm->tm_year + 1; */ + tmp = tmfc.cc + 1; + if (pg_mul_s32_overflow(tmp, 100, &tmp) || + pg_sub_s32_overflow(tmp, tm->tm_year, &tmp) || + pg_add_s32_overflow(tmp, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } } else { @@ -4839,11 +4872,31 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, if (tmfc.bc) tmfc.cc = -tmfc.cc; if (tmfc.cc >= 0) + { /* +1 because 21st century started in 2001 */ - tm->tm_year = (tmfc.cc - 1) * 100 + 1; + /* tm->tm_year = (tmfc.cc - 1) * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc - 1, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } else + { /* +1 because year == 599 is 600 BC */ - tm->tm_year = tmfc.cc * 100 + 1; + /* tm->tm_year = tmfc.cc * 100 + 1; */ + if (pg_mul_s32_overflow(tmfc.cc, 100, &tm->tm_year) || + pg_add_s32_overflow(tm->tm_year, 1, &tm->tm_year)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + text_to_cstring(date_txt), "timestamp", + escontext); + goto fail; + } + } fmask |= DTK_M(YEAR); } @@ -4868,11 +4921,35 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, fmask |= DTK_DATE_M; } else - tmfc.ddd = (tmfc.ww - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.ddd = (tmfc.ww - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.ww, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.ddd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } } if (tmfc.w) - tmfc.dd = (tmfc.w - 1) * 7 + 1; + { + int tmp = 0; + + /* tmfc.dd = (tmfc.w - 1) * 7 + 1; */ + if (pg_sub_s32_overflow(tmfc.w, 1, &tmp) || + pg_mul_s32_overflow(tmp, 7, &tmp) || + pg_add_s32_overflow(tmp, 1, &tmfc.dd)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.dd) { tm->tm_mday = tmfc.dd; @@ -4937,7 +5014,18 @@ do_to_timestamp(text *date_txt, text *fmt, Oid collid, bool std, } if (tmfc.ms) - *fsec += tmfc.ms * 1000; + { + int tmp = 0; + + /* *fsec += tmfc.ms * 1000; */ + if (pg_mul_s32_overflow(tmfc.ms, 1000, &tmp) || + pg_add_s32_overflow(*fsec, tmp, fsec)) + { + DateTimeParseError(DTERR_FIELD_OVERFLOW, NULL, + date_str, "timestamp", escontext); + goto fail; + } + } if (tmfc.us) *fsec += tmfc.us; if (fprec) diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index cdc7e43b93..5fee46a968 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -5111,6 +5111,10 @@ interval_trunc(PG_FUNCTION_ARGS) * * Return the Julian day which corresponds to the first day (Monday) of the given ISO 8601 year and week. * Julian days are used to convert between ISO week dates and Gregorian dates. + * + * XXX: This function has integer overflow hazards, but restructuring it to + * work with the soft-error handling that its callers do is likely more + * trouble than it's worth. */ int isoweek2j(int year, int week) diff --git a/src/include/common/int.h b/src/include/common/int.h index 7fc046e78a..202c1e6884 100644 --- a/src/include/common/int.h +++ b/src/include/common/int.h @@ -32,6 +32,9 @@ * - If a * b overflows, return true, otherwise store the result of a * b * into *result. The content of *result is implementation defined in case of * overflow. + * - If -a overflows, return true, otherwise store the result of -a + * into *result. The content of *result is implementation defined in case of + * overflow. *--------- */ @@ -97,6 +100,22 @@ pg_mul_s16_overflow(int16 a, int16 b, int16 *result) #endif } +static inline bool +pg_neg_s16_overflow(int16 a, int16 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT16_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT32 */ @@ -154,6 +173,22 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result) #endif } +static inline bool +pg_neg_s32_overflow(int32 a, int32 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT32_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /* * INT64 */ @@ -258,6 +293,22 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result) #endif } +static inline bool +pg_neg_s64_overflow(int64 a, int64 *result) +{ +#if defined(HAVE__BUILTIN_OP_OVERFLOW) + return __builtin_sub_overflow(0, a, result); +#else + if (unlikely(a == PG_INT64_MIN)) + { + *result = 0x5EED; /* to avoid spurious warnings */ + return true; + } + *result = -a; + return false; +#endif +} + /*------------------------------------------------------------------------ * Overflow routines for unsigned integers *------------------------------------------------------------------------ diff --git a/src/test/regress/expected/date.out b/src/test/regress/expected/date.out index 20374c5230..0e7f7534cc 100644 --- a/src/test/regress/expected/date.out +++ b/src/test/regress/expected/date.out @@ -1528,6 +1528,8 @@ select make_date(2013, 13, 1); ERROR: date field value out of range: 2013-13-01 select make_date(2013, 11, -1); ERROR: date field value out of range: 2013-11--1 +SELECT make_date(-2147483648, 1, 1); +ERROR: date field value out of range: -2147483648-01-01 select make_time(10, 55, 100.1); ERROR: time field value out of range: 10:55:100.1 select make_time(24, 0, 2.1); diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out index 82c0e1a12f..c4ac9b8145 100644 --- a/src/test/regress/expected/horology.out +++ b/src/test/regress/expected/horology.out @@ -3743,6 +3743,14 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); ERROR: date/time field value out of range: "2015-02-11 86400" +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +ERROR: value for "Y,YYY" in source string is out of range +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +ERROR: date/time field value out of range: "0.-2147483648" +SELECT to_timestamp('613566758', 'W'); +ERROR: date/time field value out of range: "613566758" +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); +ERROR: date/time field value out of range: "2024 613566758 1" SELECT to_date('2016-13-10', 'YYYY-MM-DD'); ERROR: date/time field value out of range: "2016-13-10" SELECT to_date('2016-02-30', 'YYYY-MM-DD'); @@ -3783,6 +3791,14 @@ SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be 02-01-0001 BC (1 row) +SELECT to_date('100000000', 'CC'); +ERROR: date/time field value out of range: "100000000" +SELECT to_date('-100000000', 'CC'); +ERROR: date/time field value out of range: "-100000000" +SELECT to_date('-2147483648 01', 'CC YY'); +ERROR: date/time field value out of range: "-2147483648 01" +SELECT to_date('2147483647 01', 'CC YY'); +ERROR: date/time field value out of range: "2147483647 01" -- to_char's TZ format code produces zone abbrev if known SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ'); to_char diff --git a/src/test/regress/sql/date.sql b/src/test/regress/sql/date.sql index 1c58ff6966..805aec706c 100644 --- a/src/test/regress/sql/date.sql +++ b/src/test/regress/sql/date.sql @@ -371,5 +371,6 @@ select make_date(0, 7, 15); select make_date(2013, 2, 30); select make_date(2013, 13, 1); select make_date(2013, 11, -1); +SELECT make_date(-2147483648, 1, 1); select make_time(10, 55, 100.1); select make_time(24, 0, 2.1); diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql index 0fe3c783e6..808083a6d8 100644 --- a/src/test/regress/sql/horology.sql +++ b/src/test/regress/sql/horology.sql @@ -650,6 +650,10 @@ SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSS'); SELECT to_timestamp('2015-02-11 86000', 'YYYY-MM-DD SSSSS'); -- ok SELECT to_timestamp('2015-02-11 86400', 'YYYY-MM-DD SSSSS'); +SELECT to_timestamp('1000000000,999', 'Y,YYY'); +SELECT to_timestamp('0.-2147483648', 'SS.MS'); +SELECT to_timestamp('613566758', 'W'); +SELECT to_timestamp('2024 613566758 1', 'YYYY WW D'); SELECT to_date('2016-13-10', 'YYYY-MM-DD'); SELECT to_date('2016-02-30', 'YYYY-MM-DD'); SELECT to_date('2016-02-29', 'YYYY-MM-DD'); -- ok @@ -660,6 +664,10 @@ SELECT to_date('2016 365', 'YYYY DDD'); -- ok SELECT to_date('2016 366', 'YYYY DDD'); -- ok SELECT to_date('2016 367', 'YYYY DDD'); SELECT to_date('0000-02-01','YYYY-MM-DD'); -- allowed, though it shouldn't be +SELECT to_date('100000000', 'CC'); +SELECT to_date('-100000000', 'CC'); +SELECT to_date('-2147483648 01', 'CC YY'); +SELECT to_date('2147483647 01', 'CC YY'); -- to_char's TZ format code produces zone abbrev if known SELECT to_char('2012-12-12 12:00'::timestamptz, 'YYYY-MM-DD HH:MI:SS TZ'); -- 2.39.5 (Apple Git-154)
