Tighten parsing of datetime input.

ParseFraction only expects to deal with fields that contain a decimal
point and digit(s).  However it's possible in some edge cases for it
to be passed input that doesn't look like that.  In particular the
input could look like a valid floating-point number, such as ".123e6".
strtod() will happily eat that, possibly producing a result that is
not within the expected range 0..1, which can result in integer
overflow in the callers.  That doesn't have any security consequences,
but it's still not very desirable.  Fix by checking that the input
has the expected form.

Similarly, DecodeNumberField only expects to deal with fields that
contain a decimal point and digit(s), but it's sometimes abused to
parse strings that might not look like that.  This could result in
failure to reject bogus input, yielding silly results.  Again, fix
by rejecting input that doesn't look as-expected.  That decision
also means that we can affirmatively answer the very old comment
questioning whether we couldn't save some duplicative code by
using ParseFractionalSecond here.

While these changes should only reject input that nobody would
consider valid, it still doesn't seem like a change to make in
stable branches.  Apply to HEAD only.

Reported-by: Evgeniy Gorbanev <gorbanev.es@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/1328335.1748371099@sss.pgh.pa.us

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 793d8a9adccdc43de22f1e9148d76d093f30c0f8..680fee2a8447e94f324f1b451632d431baac5343 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -702,9 +702,18 @@ ParseFraction(char *cp, double *frac)
    }
    else
    {
+       /*
+        * On the other hand, let's reject anything that's not digits after
+        * the ".".  strtod is happy with input like ".123e9", but that'd
+        * break callers' expectation that the result is in 0..1.  (It's quite
+        * difficult to get here with such input, but not impossible.)
+        */
+       if (strspn(cp + 1, "0123456789") != strlen(cp + 1))
+           return DTERR_BAD_FORMAT;
+
        errno = 0;
        *frac = strtod(cp, &cp);
-       /* check for parse failure */
+       /* check for parse failure (probably redundant given prior check) */
        if (*cp != '\0' || errno != 0)
            return DTERR_BAD_FORMAT;
    }
@@ -2958,31 +2967,28 @@ DecodeNumberField(int len, char *str, int fmask,
 {
    char       *cp;
 
+   /*
+    * This function was originally meant to cope only with DTK_NUMBER fields,
+    * but we now sometimes abuse it to parse (parts of) DTK_DATE fields,
+    * which can contain letters and other punctuation.  Reject if it's not a
+    * valid DTK_NUMBER, that is digits and decimal point(s).  (ParseFraction
+    * will reject if there's more than one decimal point.)
+    */
+   if (strspn(str, "0123456789.") != len)
+       return DTERR_BAD_FORMAT;
+
    /*
     * Have a decimal point? Then this is a date or something with a seconds
     * field...
     */
    if ((cp = strchr(str, '.')) != NULL)
    {
-       /*
-        * Can we use ParseFractionalSecond here?  Not clear whether trailing
-        * junk should be rejected ...
-        */
-       if (cp[1] == '\0')
-       {
-           /* avoid assuming that strtod will accept "." */
-           *fsec = 0;
-       }
-       else
-       {
-           double      frac;
+       int         dterr;
 
-           errno = 0;
-           frac = strtod(cp, NULL);
-           if (errno != 0)
-               return DTERR_BAD_FORMAT;
-           *fsec = rint(frac * 1000000);
-       }
+       /* Convert the fraction and store at *fsec */
+       dterr = ParseFractionalSecond(cp, fsec);
+       if (dterr)
+           return dterr;
        /* Now truncate off the fraction for further processing */
        *cp = '\0';
        len = strlen(str);

diff --git a/src/test/regress/expected/horology.out b/src/test/regress/expected/horology.out
index b90bfcd794f450fdb94c518f16ed64244c0cd8b5..5ae93d8e8a515d079360d8bca937582e6ee048ad 100644 (file)
--- a/src/test/regress/expected/horology.out
+++ b/src/test/regress/expected/horology.out
@@ -467,6 +467,15 @@ SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-08';
 ERROR:  invalid input syntax for type timestamp with time zone: "Y2001M12D27H04MM05S06.789-08"
 LINE 1: SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-0...
                                         ^
+-- More examples we used to accept and should not
+SELECT timestamp with time zone 'J2452271 T X03456-08';
+ERROR:  invalid input syntax for type timestamp with time zone: "J2452271 T X03456-08"
+LINE 1: SELECT timestamp with time zone 'J2452271 T X03456-08';
+                                        ^
+SELECT timestamp with time zone 'J2452271 T X03456.001e6-08';
+ERROR:  invalid input syntax for type timestamp with time zone: "J2452271 T X03456.001e6-08"
+LINE 1: SELECT timestamp with time zone 'J2452271 T X03456.001e6-08'...
+                                        ^
 -- conflicting fields should throw errors
 SELECT date '1995-08-06 epoch';
 ERROR:  invalid input syntax for type date: "1995-08-06 epoch"

diff --git a/src/test/regress/sql/horology.sql b/src/test/regress/sql/horology.sql
index 1310b43277380649299ee6df87378f71ba2d89bf..8978249a5dc1edabee673f23a3bcc349421172da 100644 (file)
--- a/src/test/regress/sql/horology.sql
+++ b/src/test/regress/sql/horology.sql
@@ -102,6 +102,10 @@ SELECT date 'J J 1520447';
 SELECT timestamp with time zone 'Y2001M12D27H04M05S06.789+08';
 SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-08';
 
+-- More examples we used to accept and should not
+SELECT timestamp with time zone 'J2452271 T X03456-08';
+SELECT timestamp with time zone 'J2452271 T X03456.001e6-08';
+
 -- conflicting fields should throw errors
 SELECT date '1995-08-06 epoch';
 SELECT date '1995-08-06 infinity';