Adjust datetime parsing to be more robust. We now pass the length of the

working buffer into ParseDateTime() and reject too-long input there,
rather than checking the length of the input string before calling
ParseDateTime(). The old method was bogus because ParseDateTime() can use
a variable amount of working space, depending on the content of the
input string (e.g. how many fields need to be NUL terminated). This fixes
a minor stack overrun -- I don't _think_ it's exploitable, although I
won't claim to be an expert.

Along the way, fix a bug reported by Mark Dilger: the working buffer
allocated by interval_in() was too short, which resulted in rejecting
some perfectly valid interval input values. I added a regression test for
this fix.

diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c
index 701fe1456f74a5ee480183267ee81cf8c65236f5..b47a71adb5a198e5827c98357735cdde1f0c6d5e 100644 (file)
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/date.c,v 1.93.2.2 2005/04/23 22:53:44 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/date.c,v 1.93.2.3 2005/05/26 02:14:31 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -65,12 +65,10 @@ date_in(PG_FUNCTION_ARGS)
        int                     dterr;
        char       *field[MAXDATEFIELDS];
        int                     ftype[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + 1];
+       char            workbuf[MAXDATELEN + 1];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tzp);
        if (dterr != 0)
@@ -558,15 +556,13 @@ time_in(PG_FUNCTION_ARGS)
        int                     tz;
        int                     nf;
        int                     dterr;
-       char            lowstr[MAXDATELEN + 1];
+       char            workbuf[MAXDATELEN + 1];
        char       *field[MAXDATEFIELDS];
        int                     dtype;
        int                     ftype[MAXDATEFIELDS];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
        if (dterr != 0)
@@ -1422,15 +1418,13 @@ timetz_in(PG_FUNCTION_ARGS)
        int                     tz;
        int                     nf;
        int                     dterr;
-       char            lowstr[MAXDATELEN + 1];
+       char            workbuf[MAXDATELEN + 1];
        char       *field[MAXDATEFIELDS];
        int                     dtype;
        int                     ftype[MAXDATEFIELDS];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeTimeOnly(field, ftype, nf, &dtype, tm, &fsec, &tz);
        if (dterr != 0)

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 11d293c1842766d70627457a59cfb9ca85466980..27241c31d9a11be180d353abc112183a74cbf14b 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v 1.118.2.5 2005/04/20 17:15:09 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/datetime.c,v 1.118.2.6 2005/05/26 02:14:31 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -696,21 +696,23 @@ TrimTrailingZeros(char *str)
        }
 }
 
-
 /* ParseDateTime()
  *     Break string into tokens based on a date/time context.
  *     Returns 0 if successful, DTERR code if bogus input detected.
  *
  * timestr - the input string
- * lowstr - workspace for field string storage (must be large enough for
- *             a copy of the input string, including trailing null)
+ * workbuf - workspace for field string storage. This must be
+ *   larger than the largest legal input for this datetime type --
+ *   some additional space will be needed to NUL terminate fields.
+ * buflen - the size of workbuf
  * field[] - pointers to field strings are returned in this array
  * ftype[] - field type indicators are returned in this array
  * maxfields - dimensions of the above two arrays
  * *numfields - set to the actual number of fields detected
  *
- * The fields extracted from the input are stored as separate, null-terminated
- * strings in the workspace at lowstr. Any text is converted to lower case.
+ * The fields extracted from the input are stored as separate,
+ * null-terminated strings in the workspace at workbuf. Any text is
+ * converted to lower case.
  *
  * Several field types are assigned:
  *     DTK_NUMBER - digits and (possibly) a decimal point
@@ -726,12 +728,27 @@ TrimTrailingZeros(char *str)
  *     DTK_DATE can hold Posix time zones (GMT-8)
  */
 int
-ParseDateTime(const char *timestr, char *lowstr,
+ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
                          char **field, int *ftype, int maxfields, int *numfields)
 {
        int                     nf = 0;
        const char *cp = timestr;
-       char       *lp = lowstr;
+       char       *bufp = workbuf;
+       const char *bufend = workbuf + buflen;
+
+       /*
+        * Set the character pointed-to by "bufptr" to "newchar", and
+        * increment "bufptr". "end" gives the end of the buffer -- we
+        * return an error if there is no space left to append a character
+        * to the buffer. Note that "bufptr" is evaluated twice.
+        */
+#define APPEND_CHAR(bufptr, end, newchar)              \
+       do                                                                                      \
+       {                                                                                       \
+               if (((bufptr) + 1) >= (end))                    \
+                       return DTERR_BAD_FORMAT;                        \
+               *(bufptr)++ = newchar;                                  \
+       } while (0)
 
        /* outer loop through fields */
        while (*cp != '\0')
@@ -746,23 +763,23 @@ ParseDateTime(const char *timestr, char *lowstr,
                /* Record start of current field */
                if (nf >= maxfields)
                        return DTERR_BAD_FORMAT;
-               field[nf] = lp;
+               field[nf] = bufp;
 
                /* leading digit? then date or time */
                if (isdigit((unsigned char) *cp))
                {
-                       *lp++ = *cp++;
+                       APPEND_CHAR(bufp, bufend, *cp++);
                        while (isdigit((unsigned char) *cp))
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
 
                        /* time field? */
                        if (*cp == ':')
                        {
                                ftype[nf] = DTK_TIME;
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
                                while (isdigit((unsigned char) *cp) ||
                                           (*cp == ':') || (*cp == '.'))
-                                       *lp++ = *cp++;
+                                       APPEND_CHAR(bufp, bufend, *cp++);
                        }
                        /* date field? allow embedded text month */
                        else if ((*cp == '-') || (*cp == '/') || (*cp == '.'))
@@ -770,13 +787,13 @@ ParseDateTime(const char *timestr, char *lowstr,
                                /* save delimiting character to use later */
                                char            delim = *cp;
 
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
                                /* second field is all digits? then no embedded text month */
                                if (isdigit((unsigned char) *cp))
                                {
                                        ftype[nf] = ((delim == '.') ? DTK_NUMBER : DTK_DATE);
                                        while (isdigit((unsigned char) *cp))
-                                               *lp++ = *cp++;
+                                               APPEND_CHAR(bufp, bufend, *cp++);
 
                                        /*
                                         * insist that the delimiters match to get a
@@ -785,16 +802,16 @@ ParseDateTime(const char *timestr, char *lowstr,
                                        if (*cp == delim)
                                        {
                                                ftype[nf] = DTK_DATE;
-                                               *lp++ = *cp++;
+                                               APPEND_CHAR(bufp, bufend, *cp++);
                                                while (isdigit((unsigned char) *cp) || (*cp == delim))
-                                                       *lp++ = *cp++;
+                                                       APPEND_CHAR(bufp, bufend, *cp++);
                                        }
                                }
                                else
                                {
                                        ftype[nf] = DTK_DATE;
                                        while (isalnum((unsigned char) *cp) || (*cp == delim))
-                                               *lp++ = tolower((unsigned char) *cp++);
+                                               APPEND_CHAR(bufp, bufend, tolower((unsigned char) *cp++));
                                }
                        }
 
@@ -808,9 +825,9 @@ ParseDateTime(const char *timestr, char *lowstr,
                /* Leading decimal point? Then fractional seconds... */
                else if (*cp == '.')
                {
-                       *lp++ = *cp++;
+                       APPEND_CHAR(bufp, bufend, *cp++);
                        while (isdigit((unsigned char) *cp))
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
 
                        ftype[nf] = DTK_NUMBER;
                }
@@ -822,9 +839,9 @@ ParseDateTime(const char *timestr, char *lowstr,
                else if (isalpha((unsigned char) *cp))
                {
                        ftype[nf] = DTK_STRING;
-                       *lp++ = tolower((unsigned char) *cp++);
+                       APPEND_CHAR(bufp, bufend, tolower((unsigned char) *cp++));
                        while (isalpha((unsigned char) *cp))
-                               *lp++ = tolower((unsigned char) *cp++);
+                               APPEND_CHAR(bufp, bufend, tolower((unsigned char) *cp++));
 
                        /*
                         * Full date string with leading text month? Could also be a
@@ -835,15 +852,15 @@ ParseDateTime(const char *timestr, char *lowstr,
                                char            delim = *cp;
 
                                ftype[nf] = DTK_DATE;
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
                                while (isdigit((unsigned char) *cp) || (*cp == delim))
-                                       *lp++ = *cp++;
+                                       APPEND_CHAR(bufp, bufend, *cp++);
                        }
                }
                /* sign? then special or numeric timezone */
                else if ((*cp == '+') || (*cp == '-'))
                {
-                       *lp++ = *cp++;
+                       APPEND_CHAR(bufp, bufend, *cp++);
                        /* soak up leading whitespace */
                        while (isspace((unsigned char) *cp))
                                cp++;
@@ -851,18 +868,18 @@ ParseDateTime(const char *timestr, char *lowstr,
                        if (isdigit((unsigned char) *cp))
                        {
                                ftype[nf] = DTK_TZ;
-                               *lp++ = *cp++;
+                               APPEND_CHAR(bufp, bufend, *cp++);
                                while (isdigit((unsigned char) *cp) ||
                                           (*cp == ':') || (*cp == '.'))
-                                       *lp++ = *cp++;
+                                       APPEND_CHAR(bufp, bufend, *cp++);
                        }
                        /* special? */
                        else if (isalpha((unsigned char) *cp))
                        {
                                ftype[nf] = DTK_SPECIAL;
-                               *lp++ = tolower((unsigned char) *cp++);
+                               APPEND_CHAR(bufp, bufend, tolower((unsigned char) *cp++));
                                while (isalpha((unsigned char) *cp))
-                                       *lp++ = tolower((unsigned char) *cp++);
+                                       APPEND_CHAR(bufp, bufend, tolower((unsigned char) *cp++));
                        }
                        /* otherwise something wrong... */
                        else
@@ -879,7 +896,7 @@ ParseDateTime(const char *timestr, char *lowstr,
                        return DTERR_BAD_FORMAT;
 
                /* force in a delimiter after each field */
-               *lp++ = '\0';
+               *bufp++ = '\0';
                nf++;
        }
 

diff --git a/src/backend/utils/adt/nabstime.c b/src/backend/utils/adt/nabstime.c
index 1bc96c1eb2022e810cabd180a78586dd33ec22e1..911e689200fe48d01875755a10a1f212f725856b 100644 (file)
--- a/src/backend/utils/adt/nabstime.c
+++ b/src/backend/utils/adt/nabstime.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/nabstime.c,v 1.117.2.1 2004/05/05 17:28:57 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/nabstime.c,v 1.117.2.2 2005/05/26 02:14:31 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -366,15 +366,13 @@ abstimein(PG_FUNCTION_ARGS)
                           *tm = &date;
        int                     dterr;
        char       *field[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + 1];
+       char            workbuf[MAXDATELEN + 1];
        int                     dtype;
        int                     nf,
                                ftype[MAXDATEFIELDS];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
        if (dterr != 0)
@@ -771,12 +769,10 @@ reltimein(PG_FUNCTION_ARGS)
        char       *field[MAXDATEFIELDS];
        int                     nf,
                                ftype[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + 1];
+       char            workbuf[MAXDATELEN + 1];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
        if (dterr != 0)

diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 35486b275b8a7145aa374b7aa34da2b652123f9b..4c0de321e039c695785117e621e0ba5b2253bff2 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v 1.96.2.2 2004/12/01 19:57:56 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/utils/adt/timestamp.c,v 1.96.2.3 2005/05/26 02:14:31 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -79,12 +79,10 @@ timestamp_in(PG_FUNCTION_ARGS)
        int                     dterr;
        char       *field[MAXDATEFIELDS];
        int                     ftype[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + MAXDATEFIELDS];
+       char            workbuf[MAXDATELEN + MAXDATEFIELDS];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
        if (dterr != 0)
@@ -305,12 +303,10 @@ timestamptz_in(PG_FUNCTION_ARGS)
        int                     dterr;
        char       *field[MAXDATEFIELDS];
        int                     ftype[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + MAXDATEFIELDS];
+       char            workbuf[MAXDATELEN + MAXDATEFIELDS];
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf),
+                                                 field, ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeDateTime(field, ftype, nf, &dtype, tm, &fsec, &tz);
        if (dterr != 0)
@@ -464,7 +460,7 @@ interval_in(PG_FUNCTION_ARGS)
        int                     dterr;
        char       *field[MAXDATEFIELDS];
        int                     ftype[MAXDATEFIELDS];
-       char            lowstr[MAXDATELEN + MAXDATEFIELDS];
+       char            workbuf[256];
 
        tm->tm_year = 0;
        tm->tm_mon = 0;
@@ -474,10 +470,8 @@ interval_in(PG_FUNCTION_ARGS)
        tm->tm_sec = 0;
        fsec = 0;
 
-       if (strlen(str) >= sizeof(lowstr))
-               dterr = DTERR_BAD_FORMAT;
-       else
-               dterr = ParseDateTime(str, lowstr, field, ftype, MAXDATEFIELDS, &nf);
+       dterr = ParseDateTime(str, workbuf, sizeof(workbuf), field,
+                                                 ftype, MAXDATEFIELDS, &nf);
        if (dterr == 0)
                dterr = DecodeInterval(field, ftype, nf, &dtype, tm, &fsec);
        if (dterr != 0)

diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h
index cda63ebe6de0322269c42d8eb557c068cad0616b..f0b5d876b87ac781b60fc4300d64b437d66d978c 100644 (file)
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: datetime.h,v 1.45 2003/08/27 23:29:29 tgl Exp $
+ * $Id: datetime.h,v 1.45.2.1 2005/05/26 02:14:32 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -294,7 +294,7 @@ extern void GetCurrentTimeUsec(struct tm * tm, fsec_t *fsec, int *tzp);
 extern void j2date(int jd, int *year, int *month, int *day);
 extern int     date2j(int year, int month, int day);
 
-extern int ParseDateTime(const char *timestr, char *lowstr,
+extern int ParseDateTime(const char *timestr, char *workbuf, size_t buflen,
                          char **field, int *ftype,
                          int maxfields, int *numfields);
 extern int DecodeDateTime(char **field, int *ftype,

diff --git a/src/test/regress/expected/interval.out b/src/test/regress/expected/interval.out
index 8ba178c8bf3ce2e1b6eaec2ce926d43c5d3907ed..87cbcfa428893e37b2624cccf1c02a7d6c4f8be7 100644 (file)
--- a/src/test/regress/expected/interval.out
+++ b/src/test/regress/expected/interval.out
@@ -221,3 +221,10 @@ select avg(f1) from interval_tbl;
  @ 4 years 1 mon 10 days 4 hours 18 mins 23 secs
 (1 row)
 
+-- test long interval input
+select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;
+                  interval                  
+--------------------------------------------
+ @ 4541 years 4 mons 4 days 17 mins 31 secs
+(1 row)
+

diff --git a/src/test/regress/sql/interval.sql b/src/test/regress/sql/interval.sql
index b6a6eb20ef30b1986beb00bf60c29f944ab0c670..2ae8f1f785745dab9cd6f71754b7c72124698ffa 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -66,3 +66,6 @@ SELECT '' AS ten, INTERVAL_TBL.*;
 -- updating pg_aggregate.agginitval
 
 select avg(f1) from interval_tbl;
+
+-- test long interval input
+select '4 millenniums 5 centuries 4 decades 1 year 4 months 4 days 17 minutes 31 seconds'::interval;