From 82dd5706ee5a5fc4db962fe1a672b4f71ab51612 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 12 Apr 2021 11:31:30 +0900 Subject: [PATCH] Fix out-of-bound memory access for interval -> char conversion Using Roman numbers (via "RM" or "rm") for a conversion to calculate a number of months has never considered the case of negative numbers, where a conversion could easily cause out-of-bound memory accesses. The conversions in themselves were not completely consistent either, as specifying 12 would result in NULL, but it should mean XII. This commit reworks the conversion calculation to have a more consistent behavior: - If the number of months and years is 0, return NULL. - If the number of months is positive, return the exact month number. - If the number of months is negative, do a backward calculation, with -1 meaning December, -2 November, etc. Reported-by: Theodor Arsenij Larionov-Trichkin Author: Julien Rouhaud Discussion: https://postgr.es/m/16953-f255a18f8c51f1d5@postgresql.org backpatch-through: 9.6 --- src/backend/utils/adt/formatting.c | 63 +++++++++++++++++++++---- src/test/regress/expected/timestamp.out | 36 ++++++++++++++ src/test/regress/sql/timestamp.sql | 6 +++ 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 5c54823efde..42792b7f0b6 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -2973,18 +2973,61 @@ DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid col s += strlen(s); break; case DCH_RM: - if (!tm->tm_mon) - break; - sprintf(s, "%*s", S_FM(n->suffix) ? 0 : -4, - rm_months_upper[MONTHS_PER_YEAR - tm->tm_mon]); - s += strlen(s); - break; + /* FALLTHROUGH */ case DCH_rm: - if (!tm->tm_mon) + + /* + * For intervals, values like '12 month' will be reduced to 0 + * month and some years. These should be processed. + */ + if (!tm->tm_mon && !tm->tm_year) break; - sprintf(s, "%*s", S_FM(n->suffix) ? 0 : -4, - rm_months_lower[MONTHS_PER_YEAR - tm->tm_mon]); - s += strlen(s); + else + { + int mon = 0; + const char *const *months; + + if (n->key->id == DCH_RM) + months = rm_months_upper; + else + months = rm_months_lower; + + /* + * Compute the position in the roman-numeral array. Note + * that the contents of the array are reversed, December + * being first and January last. + */ + if (tm->tm_mon == 0) + { + /* + * This case is special, and tracks the case of full + * interval years. + */ + mon = tm->tm_year >= 0 ? 0 : MONTHS_PER_YEAR - 1; + } + else if (tm->tm_mon < 0) + { + /* + * Negative case. In this case, the calculation is + * reversed, where -1 means December, -2 November, + * etc. + */ + mon = -1 * (tm->tm_mon + 1); + } + else + { + /* + * Common case, with a strictly positive value. The + * position in the array matches with the value of + * tm_mon. + */ + mon = MONTHS_PER_YEAR - tm->tm_mon; + } + + sprintf(s, "%*s", S_FM(n->suffix) ? 0 : -4, + months[mon]); + s += strlen(s); + } break; case DCH_W: sprintf(s, "%d", (tm->tm_mday - 1) / 7 + 1); diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out index ba27d9e2c5d..3025d8431e0 100644 --- a/src/test/regress/expected/timestamp.out +++ b/src/test/regress/expected/timestamp.out @@ -1688,6 +1688,42 @@ SELECT '' AS to_char_11, to_char(d1, 'FMIYYY FMIYY FMIY FMI FMIW FMIDDD FMID') | 2001 1 1 1 1 1 1 (65 rows) +-- Roman months, with upper and lower case. +SELECT i, + to_char(i * interval '1mon', 'rm'), + to_char(i * interval '1mon', 'RM') + FROM generate_series(-13, 13) i; + i | to_char | to_char +-----+---------+--------- + -13 | xii | XII + -12 | i | I + -11 | ii | II + -10 | iii | III + -9 | iv | IV + -8 | v | V + -7 | vi | VI + -6 | vii | VII + -5 | viii | VIII + -4 | ix | IX + -3 | x | X + -2 | xi | XI + -1 | xii | XII + 0 | | + 1 | i | I + 2 | ii | II + 3 | iii | III + 4 | iv | IV + 5 | v | V + 6 | vi | VI + 7 | vii | VII + 8 | viii | VIII + 9 | ix | IX + 10 | x | X + 11 | xi | XI + 12 | xii | XII + 13 | i | I +(27 rows) + -- timestamp numeric fields constructor SELECT make_timestamp(2014,12,28,6,30,45.887); make_timestamp diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql index 56d57848f9d..55380c59e77 100644 --- a/src/test/regress/sql/timestamp.sql +++ b/src/test/regress/sql/timestamp.sql @@ -231,5 +231,11 @@ SELECT '' AS to_char_10, to_char(d1, 'IYYY IYY IY I IW IDDD ID') SELECT '' AS to_char_11, to_char(d1, 'FMIYYY FMIYY FMIY FMI FMIW FMIDDD FMID') FROM TIMESTAMP_TBL; +-- Roman months, with upper and lower case. +SELECT i, + to_char(i * interval '1mon', 'rm'), + to_char(i * interval '1mon', 'RM') + FROM generate_series(-13, 13) i; + -- timestamp numeric fields constructor SELECT make_timestamp(2014,12,28,6,30,45.887);