Re: Returning non-terminated string in ECPG Informix-compatible function

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Oleg Tselebrovskiy <o(dot)tselebrovskiy(at)postgrespro(dot)ru>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Returning non-terminated string in ECPG Informix-compatible function
Date: 2024-02-15 05:25:07
Message-ID: Zc2gMx75UgbBKvLt@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Feb 15, 2024 at 12:15:40PM +0700, Oleg Tselebrovskiy wrote:
> Greetings again.
> I was looking through more static analyzer output and found another problem.
> In ecpg/pgtypeslib/dt_common.c there are 4 calls of pgtypes_alloc.
> This function uses calloc and returns NULL if OOM, but we don't check its
> return value and immediately pass it to strcpy, which could lead to
> segfault.
>
> I suggest adding a check for a return value since all other calls of
> pgtypes_alloc are checked for NULL.

Right.

> @@ -654,7 +654,7 @@ intoasc(interval * i, char *str)
> if (!tmp)
> return -errno;
>
> - memcpy(str, tmp, strlen(tmp));
> + strcpy(str, tmp);

For this stuff, Ashutosh's point was to integrate a test case in the
patch. With the pgc you have posted, most of the work is done, but
this should be added to src/interfaces/ecpg/test/sql/ to add some
automated coverage. See the area for references showing how this is
achieved.

> @@ -2837,6 +2843,8 @@ PGTYPEStimestamp_defmt_scan(char **str, char *fmt, timestamp * d,
> case 'T':
> pfmt++;
> tmp = pgtypes_alloc(strlen("%H:%M:%S") + strlen(pstr) + 1);
> + if(!tmp)
> + return 1;

These are obviously wrong as pgtypes_alloc() could return NULL.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shubham Khanna 2024-02-15 05:27:48 Re: speed up a logical replica setup
Previous Message Oleg Tselebrovskiy 2024-02-15 05:18:33 xmlBufferCreate return value not checked in pgxmlNodeSetToText