Skip to content

Commit e5d64fd

Browse files
tglsfdctglsfdc
committed
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
1 parent be86ca1 commit e5d64fd

File tree

3 files changed

+38
-19
lines changed

3 files changed

+38
-19
lines changed

‎src/backend/utils/adt/datetime.c

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -702,9 +702,18 @@ ParseFraction(char *cp, double *frac)
702702
}
703703
else
704704
{
705+
/*
706+
* On the other hand, let's reject anything that's not digits after
707+
* the ".". strtod is happy with input like ".123e9", but that'd
708+
* break callers' expectation that the result is in 0..1. (It's quite
709+
* difficult to get here with such input, but not impossible.)
710+
*/
711+
if (strspn(cp + 1, "0123456789") != strlen(cp + 1))
712+
return DTERR_BAD_FORMAT;
713+
705714
errno = 0;
706715
*frac = strtod(cp, &cp);
707-
/* check for parse failure */
716+
/* check for parse failure (probably redundant given prior check) */
708717
if (*cp != '\0' || errno != 0)
709718
return DTERR_BAD_FORMAT;
710719
}
@@ -2958,31 +2967,28 @@ DecodeNumberField(int len, char *str, int fmask,
29582967
{
29592968
char *cp;
29602969

2970+
/*
2971+
* This function was originally meant to cope only with DTK_NUMBER fields,
2972+
* but we now sometimes abuse it to parse (parts of) DTK_DATE fields,
2973+
* which can contain letters and other punctuation. Reject if it's not a
2974+
* valid DTK_NUMBER, that is digits and decimal point(s). (ParseFraction
2975+
* will reject if there's more than one decimal point.)
2976+
*/
2977+
if (strspn(str, "0123456789.") != len)
2978+
return DTERR_BAD_FORMAT;
2979+
29612980
/*
29622981
* Have a decimal point? Then this is a date or something with a seconds
29632982
* field...
29642983
*/
29652984
if ((cp = strchr(str, '.')) != NULL)
29662985
{
2967-
/*
2968-
* Can we use ParseFractionalSecond here? Not clear whether trailing
2969-
* junk should be rejected ...
2970-
*/
2971-
if (cp[1] == '\0')
2972-
{
2973-
/* avoid assuming that strtod will accept "." */
2974-
*fsec = 0;
2975-
}
2976-
else
2977-
{
2978-
double frac;
2986+
int dterr;
29792987

2980-
errno = 0;
2981-
frac = strtod(cp, NULL);
2982-
if (errno != 0)
2983-
return DTERR_BAD_FORMAT;
2984-
*fsec = rint(frac * 1000000);
2985-
}
2988+
/* Convert the fraction and store at *fsec */
2989+
dterr = ParseFractionalSecond(cp, fsec);
2990+
if (dterr)
2991+
return dterr;
29862992
/* Now truncate off the fraction for further processing */
29872993
*cp = '\0';
29882994
len = strlen(str);

‎src/test/regress/expected/horology.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,15 @@ SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-08';
467467
ERROR: invalid input syntax for type timestamp with time zone: "Y2001M12D27H04MM05S06.789-08"
468468
LINE 1: SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-0...
469469
^
470+
-- More examples we used to accept and should not
471+
SELECT timestamp with time zone 'J2452271 T X03456-08';
472+
ERROR: invalid input syntax for type timestamp with time zone: "J2452271 T X03456-08"
473+
LINE 1: SELECT timestamp with time zone 'J2452271 T X03456-08';
474+
^
475+
SELECT timestamp with time zone 'J2452271 T X03456.001e6-08';
476+
ERROR: invalid input syntax for type timestamp with time zone: "J2452271 T X03456.001e6-08"
477+
LINE 1: SELECT timestamp with time zone 'J2452271 T X03456.001e6-08'...
478+
^
470479
-- conflicting fields should throw errors
471480
SELECT date '1995-08-06 epoch';
472481
ERROR: invalid input syntax for type date: "1995-08-06 epoch"

‎src/test/regress/sql/horology.sql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ SELECT date 'J J 1520447';
102102
SELECT timestamp with time zone 'Y2001M12D27H04M05S06.789+08';
103103
SELECT timestamp with time zone 'Y2001M12D27H04MM05S06.789-08';
104104

105+
-- More examples we used to accept and should not
106+
SELECT timestamp with time zone 'J2452271 T X03456-08';
107+
SELECT timestamp with time zone 'J2452271 T X03456.001e6-08';
108+
105109
-- conflicting fields should throw errors
106110
SELECT date '1995-08-06 epoch';
107111
SELECT date '1995-08-06 infinity';

0 commit comments

Comments
 (0)