Re: [PATCH] Fix buffer not null terminated on (ecpg lib)

Em sex., 11 de jun. de 2021 às 19:49, Andres Freund <andres(at)anarazel(dot)de>
escreveu:

> Hi,
>
> On 2020-04-23 14:36:15 +0900, Kyotaro Horiguchi wrote:
> > At Thu, 23 Apr 2020 01:21:21 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
> wrote in
> > > Em qua., 22 de abr. de 2020 às 23:27, Kyotaro Horiguchi <
> > > horikyota(dot)ntt(at)gmail(dot)com> escreveu:
> > > >
> > > > - strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > > > sizeof(sqlca->sqlerrm.sqlerrmc));
> > > > - sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1]
> = 0;
> > > > + sqlca->sqlerrm.sqlerrmc[sizeof(sqlca->sqlerrm.sqlerrmc) - 1]
> =
> > > > '\0';
> > > > + strncpy(sqlca->sqlerrm.sqlerrmc, message,
> > > > sizeof(sqlca->sqlerrm.sqlerrmc) - 1);
> > > >
> > > > The existing strncpy then terminating by NUL works fine. I don't
> think
> > > > there's any point in doing the reverse way. Actually
> > > > sizeof(sqlca->sqlerrm.sqlerrmc) - 1 is enough for the length but the
> > > > existing code is not necessarily a bug.
> > > >
> > > Without understanding then, why Coveriy claims bug here.
> >
> > Well, handling non-terminated strings with str* functions are a sign
> > of bug in most cases. Coverity is very useful but false positives are
> > annoying. I wonder what if we attach Coverity annotations to such
> > codes.
>
> It might be worth doing something about this, for other reasons. We have
> disabled -Wstringop-truncation in 716585235b1. But I've enabled it in my
> debug build, because I find it useful. The only warning we're getting
> in non-optimized builds is
>
> /home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c: In
> function ‘ECPGset_var’:
> /home/andres/src/postgresql/src/interfaces/ecpg/ecpglib/misc.c:565:17:
> warning: ‘strncpy’ output truncated before terminating nul copying 5 bytes
> from a string of the same length [-Wstringop-truncation]
> 565 | strncpy(sqlca->sqlstate, "YE001",
> sizeof(sqlca->sqlstate));
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
memcpy would not suffer from it?

regards,
Ranier Vilela