Re: date_in and buffer overrun

It seems date_in() has a risk of buffer overrun. If the input is '.',
it sets field[0] to the beginning of workbuf and goes into
DecodeDate(). This function checks null-termination of the head of
string, but it can go beyond the end of string inside the first loop
and replace some bytes with zero. The worst scenario we've seen is
overwrite of the stack frame, in which the compiler rearranged the
memory allocation of local variables in date_in() and work_buf is at
lower address than field.

I tried to attach a patch file but failed somehow, so I paste the fix here.

Thanks,

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index d827d7d..b81960a 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -2176,9 +2176,13 @@ DecodeDate(char *str, int fmask, int *tmask,
bool *is2digits,
while (*str != '\0' && nf < MAXDATEFIELDS)
{
/* skip field separators */
- while (!isalnum((unsigned char) *str))
+ while (*str != '\0' && !isalnum((unsigned char) *str))
str++;

+ /* or it may not be what we expected... */
+ if (*str == '\0')
+ return DTERR_BAD_FORMAT;
+
field[nf] = str;
if (isdigit((unsigned char) *str))
{

--
Hitoshi Harada

On Mon, Oct 1, 2012 at 3:30 PM, Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> wrote:
> lower address than field.

Ugh, s/lower/higher/

--
Hitoshi Harada

On 02.10.2012 01:30, Hitoshi Harada wrote:
> It seems date_in() has a risk of buffer overrun. If the input is '.',
> it sets field[0] to the beginning of workbuf and goes into
> DecodeDate(). This function checks null-termination of the head of
> string, but it can go beyond the end of string inside the first loop
> and replace some bytes with zero. The worst scenario we've seen is
> overwrite of the stack frame, in which the compiler rearranged the
> memory allocation of local variables in date_in() and work_buf is at
> lower address than field.
>
> I tried to attach a patch file but failed somehow, so I paste the fix here.

Thanks, applied to master and all supported back-branches.

- Heikki