<div dir="ltr">Zombie revive:<div><br></div><div>I was checking a different project with clang-analyzer which reminded me of this.</div><div><br></div><div>Line in question that it complained about was this one: </div><div><br></div><div>strncat (build_opts_new, " -cl-opt-disable", sizeof (build_opts_new) - 1);<br></div><div><br></div><div>The error was:</div><div><br></div><div>Potential buffer overflow. Replace with 'sizeof(build_opts_new) - strlen(build_opts_new) - 1' or use a safer 'strlcat' API<br></div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Oct 5, 2017 at 1:46 AM Pali Rohár <<a href="mailto:pali.rohar@gmail.com">pali.rohar@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thursday 05 October 2017 01:41:02 <a href="mailto:rosenp@gmail.com" target="_blank">rosenp@gmail.com</a> wrote:<br>
> On Thu, 2017-10-05 at 09:48 +0200, Pali Rohár wrote:<br>
> > On Wednesday 04 October 2017 19:22:11 Rosen Penev wrote:<br>
> > > diff --git a/src/cache.c b/src/cache.c<br>
> > > index 4f43246..88851e7 100644<br>
> > > --- a/src/cache.c<br>
> > > +++ b/src/cache.c<br>
> > > @@ -572,7 +572,7 @@ struct crec *cache_insert(char *name, struct<br>
> > > all_addr *addr,<br>
> > >      }<br>
> > ><br>
> > >    if (name)<br>
> > > -    strcpy(cache_get_name(new), name);<br>
> > > +    strncpy(cache_get_name(new), name,<br>
> > > strlen(cache_get_name(new)));<br>
> ><br>
> > Hi! This line looks suspicious. Should not be length argument sizeof<br>
> > of<br>
> > destination buffer, instead of current length of null term string?<br>
> ><br>
><br>
> Doesn't sizeof on a pointer return the size of the pointer instead of<br>
> the string?<br>
<br>
Yes! Therefore I wrote word construction "size of destination buffer"<br>
and not code "sizeof(buf)". You need to take size of that destination<br>
buffer manually.<br>
<br>
> > Also strncpy in some circumstances fill string which is not null<br>
> > terminated.<br>
><br>
> strlen apparently does not include \0. needs a + 1 looks like.<br>
<br>
That is another problem. But still my above sentence about strncpy<br>
applies. strncpy does not ensure that destination string would be always<br>
null terminated, even source one was.<br>
<br>
--<br>
Pali Rohár<br>
<a href="mailto:pali.rohar@gmail.com" target="_blank">pali.rohar@gmail.com</a><br>
</blockquote></div>