Обсуждение: SQLGetInfo Bug

I've been working with Didier Moens to try to fix the bug detailed at
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=80394.

As I said towards the end, the error is caused by an overflow of the tmp buffer in PGAPI_GetInfo in info.c for which I
havecommitted a change doubling it's size to 256 bytes. I also tried using snprintf instead of sprintf for safety: 

snprintf(tmp, sizeof(tmp) - 1, "%s %s", POSTGRESDRIVERVERSION, conn->pg_version);

but somewhere along the way garbage is getting appended back on the string returned:

[328][SQLGetInfo][328]PGAPI_GetInfo: entering...fInfoType=18
[328]PGAPI_GetInfo: p='07.02.0005 PostgreSQL 7.3 on i386-redhat-linux-gnu, compiled by GCC i386-redhat-linux-gcc (GCC)
3.2.120021207 (Red Hat Linux 8wãF¸8Ù
', len=0, value=0, cbMax=0 

I can't see what the heck I've done wrong here. Any ideas?

Cheers, Dave.

"Dave Page" <dpage@vale-housing.co.uk> writes:
> snprintf(tmp, sizeof(tmp) - 1, "%s %s", POSTGRESDRIVERVERSION, conn->pg_version);

> but somewhere along the way garbage is getting appended back on the string returned:

> [328][SQLGetInfo][328]PGAPI_GetInfo: entering...fInfoType=18
> [328]PGAPI_GetInfo: p='07.02.0005 PostgreSQL 7.3 on i386-redhat-linux-gnu, compiled by GCC i386-redhat-linux-gcc
(GCC)3.2.1 20021207 (Red Hat Linux 8w�F�8�
', len=0, value=0, cbMax=0 

> I can't see what the heck I've done wrong here. Any ideas?

I think the error is elsewhere, and that the garbage is already present
in conn->pg_version.

            regards, tom lane

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: 02 January 2003 14:36
> To: Dave Page
> Cc: pgsql-odbc@postgresql.org; Didier Moens
> Subject: Re: [ODBC] SQLGetInfo Bug
>
>
> "Dave Page" <dpage@vale-housing.co.uk> writes:
> > snprintf(tmp, sizeof(tmp) - 1, "%s %s", POSTGRESDRIVERVERSION,
> > conn->pg_version);
>
> > but somewhere along the way garbage is getting appended back on the
> > string returned:
>
> > [328][SQLGetInfo][328]PGAPI_GetInfo: entering...fInfoType=18
> > [328]PGAPI_GetInfo: p='07.02.0005 PostgreSQL 7.3 on
> > i386-redhat-linux-gnu, compiled by GCC i386-redhat-linux-gcc (GCC)
> > 3.2.1 20021207 (Red Hat Linux 8wãF¸8Ù
', len=0, value=0, cbMax=0
>
> > I can't see what the heck I've done wrong here. Any ideas?
>
> I think the error is elsewhere, and that the garbage is
> already present in conn->pg_version.

Hi Tom,

The garbage is not there if I use sprintf with a larger size of tmp. I've tried variations of the code to check
(withoutluck) but I assume I'm not somehow knocking the null off the end of the string? 

Also of note, is that the logging call from which I'm seeing the garbage occurs before any other manipulation of the
string.

Regards, Dave.

"Dave Page" <dpage@vale-housing.co.uk> writes:
> The garbage is not there if I use sprintf with a larger size of
> tmp. I've tried variations of the code to check (without luck) but I
> assume I'm not somehow knocking the null off the end of the string?

You would be losing the trailing null if the snprintf is unable to fit
its output into the buffer size it's told to use (at least, that's how
most implementations of snprintf react, I think).

It doesn't look like this should be enough data to overrun your buffer,
but I don't have a better theory at the moment.

Perhaps

    snprintf(tmp, sizeof(tmp) - 1, "%s %s", ...);
    tmp[sizeof(tmp) - 1] = '\0';

would be better coding practice if you don't want to actually check
snprintf's result.

            regards, tom lane

> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: 02 January 2003 15:03
> To: Dave Page
> Cc: pgsql-odbc@postgresql.org; Didier Moens
> Subject: Re: [ODBC] SQLGetInfo Bug
>
>
> "Dave Page" <dpage@vale-housing.co.uk> writes:
> > The garbage is not there if I use sprintf with a larger
> size of tmp.
> > I've tried variations of the code to check (without luck)
> but I assume
> > I'm not somehow knocking the null off the end of the string?
>
> You would be losing the trailing null if the snprintf is
> unable to fit its output into the buffer size it's told to
> use (at least, that's how most implementations of snprintf
> react, I think).
>
> It doesn't look like this should be enough data to overrun
> your buffer, but I don't have a better theory at the moment.
>
> Perhaps
>
>     snprintf(tmp, sizeof(tmp) - 1, "%s %s", ...);
>     tmp[sizeof(tmp) - 1] = '\0';
>
> would be better coding practice if you don't want to actually
> check snprintf's result.

Thanks Tom, that got it.

Regards, Dave.