From 63e0d612f5a53d76218d4e59a35287391e284561 Mon Sep 17 00:00:00 2001 From: Neil Conway Date: Thu, 26 May 2005 02:04:14 +0000 Subject: [PATCH] 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. --- src/backend/utils/adt/date.c | 26 ++++----- src/backend/utils/adt/datetime.c | 77 ++++++++++++++++---------- src/backend/utils/adt/nabstime.c | 18 +++--- src/backend/utils/adt/timestamp.c | 26 ++++----- src/include/utils/datetime.h | 4 +- src/test/regress/expected/interval.out | 7 +++ src/test/regress/sql/interval.sql | 3 + 7 files changed, 86 insertions(+), 75 deletions(-) diff --git a/src/backend/utils/adt/date.c b/src/backend/utils/adt/date.c index 1e54f3877e..5371c64250 100644 --- 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 bb70f8d3aa..509a89f860 100644 --- 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 46483aa598..bebd3f9d32 100644 --- 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 9aa28ffcc6..27e0523f49 100644 --- 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 532cfb9206..47817d1b34 100644 --- 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 ab89b2232d..daa6348785 100644 --- 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 a05d818a33..cd17ebbc95 100644 --- 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;