Re: [dev] [sbase] [patch] Fix warnings about strcpy() etc. on OpenBSD

From: sin <sin_AT_2f30.org>
Date: Thu, 15 Aug 2013 14:42:59 +0300

On Thu, Aug 15, 2013 at 11:34:32AM +0000, Thorsten Glaser wrote:
> sin dixit:
>
> >On Thu, Aug 15, 2013 at 11:00:11AM +0000, Thorsten Glaser wrote:
> >
> >> > if(len+1 > *size && !(*p = realloc(*p, len+1)))
> ^^^^^
>
> >> > eprintf("realloc:");
> >> >
> >> >- strcpy(&(*p)[len-n], buf);
> >> >+ snprintf(&(*p)[len-n], n+1, "%s", buf);
> >>
> >> Again, I object… you do not calculate the length correctly.
> >> Besides, this looks like a strlcat to me… if not, memcpy
> >> might again be more wise; n+1 doesn’t match with len+1 from above.
> >
> >Will change these to memcpy(), thanks. However, I don't understand why n + 1
> >is wrong here?
>
> p is allocated with “len+1” bytes, so “len+1” should show up in
> its size calculations… I admit (len+1-(len-n)) factors out as
> n+1, but let the compiler do that was said recently AFAICT?
>
> Anyway, this kind of offset magic is prone to produce buffer
> over-/underflows later when other people touch the code.
> Better be explicit: if you want a strcat, use strlcat.

I definitely agree with you.

Thanks,
sin
Received on Thu Aug 15 2013 - 13:42:59 CEST