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

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index d4c773e422f630687575e16321691f00dfe90898..75a937363a10365eaf34045ee1f0f6142aa3793e 100644 (file)
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -2896,18 +2896,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 f1e17be28d7efd07aa0b40b77c7bac7ad6200b76..06d32e221befa1c1ae2135ff0cba263bc8741972 100644 (file)
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -1701,6 +1701,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 9f3f0bbb4f7dc18bb62734060020d6b93f5d7e91..2cd4e769d87cef442f6ba1545b7b0c7016502ad0 100644 (file)
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -235,5 +235,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);