[Dnsmasq-discuss] [PATCH] Fix some errors and warnings from clang-analyzer

Rosen Penev rosenp at gmail.com
Fri Oct 20 04:02:16 BST 2017


Zombie revive:

I was checking a different project with clang-analyzer which reminded me of
this.

Line in question that it complained about was this one:

strncat (build_opts_new, " -cl-opt-disable", sizeof (build_opts_new) - 1);

The error was:

Potential buffer overflow. Replace with 'sizeof(build_opts_new) -
strlen(build_opts_new) - 1' or use a safer 'strlcat' API

On Thu, Oct 5, 2017 at 1:46 AM Pali Rohár <pali.rohar at gmail.com> wrote:

> On Thursday 05 October 2017 01:41:02 rosenp at gmail.com wrote:
> > On Thu, 2017-10-05 at 09:48 +0200, Pali Rohár wrote:
> > > On Wednesday 04 October 2017 19:22:11 Rosen Penev wrote:
> > > > diff --git a/src/cache.c b/src/cache.c
> > > > index 4f43246..88851e7 100644
> > > > --- a/src/cache.c
> > > > +++ b/src/cache.c
> > > > @@ -572,7 +572,7 @@ struct crec *cache_insert(char *name, struct
> > > > all_addr *addr,
> > > >      }
> > > >
> > > >    if (name)
> > > > -    strcpy(cache_get_name(new), name);
> > > > +    strncpy(cache_get_name(new), name,
> > > > strlen(cache_get_name(new)));
> > >
> > > Hi! This line looks suspicious. Should not be length argument sizeof
> > > of
> > > destination buffer, instead of current length of null term string?
> > >
> >
> > Doesn't sizeof on a pointer return the size of the pointer instead of
> > the string?
>
> Yes! Therefore I wrote word construction "size of destination buffer"
> and not code "sizeof(buf)". You need to take size of that destination
> buffer manually.
>
> > > Also strncpy in some circumstances fill string which is not null
> > > terminated.
> >
> > strlen apparently does not include \0. needs a + 1 looks like.
>
> That is another problem. But still my above sentence about strncpy
> applies. strncpy does not ensure that destination string would be always
> null terminated, even source one was.
>
> --
> Pali Rohár
> pali.rohar at gmail.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20171020/8000989b/attachment.html>


More information about the Dnsmasq-discuss mailing list