Re: strncpy is not a safe version of strcpy

On Fri, Nov 15, 2013 at 12:33 PM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:

> > It is likely far better explained here -->
> > http://www.courtesan.com/todd/papers/strlcpy.html
> >
> > For example , the following 2 lines in jsonfuncs.c
> >
> > memset(name, 0, NAMEDATALEN);
> > strncpy(name, fname, NAMEDATALEN);
>
> Be careful with 'Name' data type - it's not just a simple string buffer.
> AFAIK it needs to work with hashing etc. so the zeroing is actually needed
> here to make sure two values produce the same result. At least that's how
> I understand the code after a quick check - for example this is from the
> same jsonfuncs.c you mentioned:
>
> memset(fname, 0, NAMEDATALEN);
> strncpy(fname, NameStr(tupdesc->attrs[i]->attname), NAMEDATALEN);
> hashentry = hash_search(json_hash, fname, HASH_FIND, NULL);
>
> So the zeroing is on purpose, although if strncpy does that then the
> memset is probably superflous. Either people do that because of habit /
> copy'n'paste, or maybe there are supported platforms when strncpy does not
> behave like this for some reason.
>
>
I had not thought of the fact the some platforms don't properly implement
strncpy(). On quick check http://man.he.net/man3/strncpy seems to indicate
that this behaviour is part of the C89 standard. So does this mean we can
always assume that all supported platforms always 0 out the remaining
buffer?

> I seriously doubt this inefficiency is going to be measurable in real
> world. If the result was a buffer-overflow bug, that'd be a different
> story, but maybe we could check the ~120 calls to strncpy in the whole
> code base and replace it with strlcpy where appropriate.
>
>
The example was more of a demonstration of wrong assumption rather than
wasted cycles. Though the wasted cycles was on my mind a bit too. I was
more focused on trying to draw a bit of attention to commit
061b88c732952c59741374806e1e41c1ec845d50 which uses strncpy and does not
properly set the last byte to 0 afterwards. I think this case could just be
replaced with strlcpy which does all this hard work for us.

Regards

David Rowley

> That being said, thanks for looking into things like this.
>
> Tomas
>
>