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 1e54f3877e8b4f01ac6c3e2ca496e3c48efdd3ec..5371c64250a7b24f5763c3a108a174f5dd5ba477 100644 (file)
--- a/src/backend/utils/adt/date.c
+++ b/src/backend/utils/adt/date.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/date.c,v 1.108 2005/05/24 02:09:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/date.c,v 1.109 2005/05/26 02:04:13 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)
@@ -894,15 +892,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)
@@ -1733,15 +1729,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 bb70f8d3aaae81bde54b708931b044cbe3210a56..509a89f8602d9be5e2bc18bb2eba7984c3584f77 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/datetime.c,v 1.144 2005/05/24 02:09:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/datetime.c,v 1.145 2005/05/26 02:04:13 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -699,21 +699,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
@@ -729,12 +731,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')
@@ -749,23 +766,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 == '.')
@@ -773,13 +790,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
@@ -788,16 +805,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++ = pg_tolower((unsigned char) *cp++);
+                       APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
                }
            }
 
@@ -811,9 +828,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;
        }
@@ -825,9 +842,9 @@ ParseDateTime(const char *timestr, char *lowstr,
        else if (isalpha((unsigned char) *cp))
        {
            ftype[nf] = DTK_STRING;
-           *lp++ = pg_tolower((unsigned char) *cp++);
+           APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
            while (isalpha((unsigned char) *cp))
-               *lp++ = pg_tolower((unsigned char) *cp++);
+               APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
 
            /*
             * Full date string with leading text month? Could also be a
@@ -838,15 +855,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++;
@@ -854,18 +871,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++ = pg_tolower((unsigned char) *cp++);
+               APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
                while (isalpha((unsigned char) *cp))
-                   *lp++ = pg_tolower((unsigned char) *cp++);
+                   APPEND_CHAR(bufp, bufend, pg_tolower((unsigned char) *cp++));
            }
            /* otherwise something wrong... */
            else
@@ -882,7 +899,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 46483aa5981e32b9ace98e58dfa30993acdf9e33..bebd3f9d3256096719094b43502e3cf8341cd2ef 100644 (file)
--- a/src/backend/utils/adt/nabstime.c
+++ b/src/backend/utils/adt/nabstime.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/nabstime.c,v 1.131 2005/05/24 02:09:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/nabstime.c,v 1.132 2005/05/26 02:04:13 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -306,15 +306,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)
@@ -711,12 +709,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 9aa28ffcc6a97938071ecc6f4bed583e53ed3e41..27e0523f49cc1884bdaea41a1f54c4fe4316fdc6 100644 (file)
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.123 2005/05/24 02:09:45 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/utils/adt/timestamp.c,v 1.124 2005/05/26 02:04:13 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -77,12 +77,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)
@@ -317,12 +315,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)
@@ -493,7 +489,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;
@@ -503,10 +499,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 532cfb92062fbb867b895ddb0cecc01999ebb012..47817d1b3485a50bc22c55e6bd81cff76faf1641 100644 (file)
--- a/src/include/utils/datetime.h
+++ b/src/include/utils/datetime.h
@@ -9,7 +9,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/utils/datetime.h,v 1.53 2005/05/24 04:03:01 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/utils/datetime.h,v 1.54 2005/05/26 02:04:14 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -276,7 +276,7 @@ extern void GetCurrentTimeUsec(struct pg_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 ab89b2232d595c9562801f89d51ce9965eeb7df3..daa6348785a9fa66fb534f9d4f711c977ce57804 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 a05d818a330fbf5ad07705e0b8276088e74fad79..cd17ebbc95ba1ed88303d0507eb1f6cbce6ce6b3 100644 (file)
--- a/src/test/regress/sql/interval.sql
+++ b/src/test/regress/sql/interval.sql
@@ -66,3 +66,6 @@ SELECT '' AS ten, * FROM 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;