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 783c7b5e7acf91410fa63f52406bbbd1a73cc1d5..a1145e2721db9f52b5e084a2f568d11e5ec86a4a 100644 (file)
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -3207,18 +3207,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 ac1a4a9b6acad6c46b2cb2927438cce2cdf557ae..6fdca6b295cbab77ba1f91880d922397727ba4bb 100644 (file)
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -1961,6 +1961,42 @@ SELECT to_char(d, 'FF1 FF2 FF3 FF4 FF5 FF6  ff1 ff2 ff3 ff4 ff5 ff6  MS US')
  7 78 789 7890 78901 789012  7 78 789 7890 78901 789012  789 789012
 (4 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 d51e83127aee96ceb620c2f29db62ab67e7ec72c..2841d2f2af462c76808f870ddd75eb1918260659 100644 (file)
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -353,6 +353,12 @@ SELECT to_char(d, 'FF1 FF2 FF3 FF4 FF5 FF6  ff1 ff2 ff3 ff4 ff5 ff6  MS US')
        ('2018-11-02 12:34:56.78901234')
    ) d(d);
 
+-- 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);
 SELECT make_timestamp(-44, 3, 15, 12, 30, 15);