Re: [hackers] [dwm] [PATCH] Replace str[n]cpy with strlcpy

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Tue, 7 Jun 2016 19:31:48 +0200

On Mon, Jun 06, 2016 at 08:50:31PM +0200, FRIGN wrote:
> On Mon, 6 Jun 2016 20:12:19 +0200
> k0ga_AT_shike2.com wrote:
>
>> ...
>
> Nope, it's not. Keep in mind strlcpy fills the rest
> of the memory area with 0's.

No it doesn't, but strncpy does that.

> Also, I suspect you have not understood strlcpy()
> at all. Read the manpage!

Roberto wrote compilers before you were a baby.

> The size-argument of strlcpy is wrt to the size of
> the _destination_, not the _source_.
> Your example also hides the fact that "nsrc" has to
> be first determined with strlen().
>

It depends, strlcpy copies the data regardless if truncation occurs, so it is
slower in some cases. It often does not matter though.

> > From my point of view the worst thing is that people believe
> > that using strlcpy the code magically becomes secure, and this
> > is a totally false security sensation. You have to check the
> > return code, and it means that the code is totally equivalent
> > to an explicit if. Look for example this case:
> >
> > deluser(strlcpy(dst, "user15", 4));
> >

Truncation needs to be checked regardless if strlcpy is used or not, imo the
pattern of using strlcpy is nice in alot (but not all) cases. It is usually:

        if (strlcpy(buf, s, sizeof(buf)) >= sizeof(buf))
                ...;

> You cannot possibly check these things by yourself. What
> strlcpy does is give you an insurance in case there is an
> overflow that the program doesn't pass pointers to
> unterminated strings around. Of course you have to check
> the return value anyway.
> I don't know why you are always riding the same horse
> here given that:
> 1) dwm used strcpy (!)
> 2) dwm used _unchecked_ strncpy (!)
> so you definitely should calm down a bit here.
>

Dwm is not insecure in these cases, it is more a style issue. Some of the
strncpy cases can be replaced even with strcpy, it can't overflow unless
the user screws up his config.

-- 
Kind regards,
Hiltjo
Received on Tue Jun 07 2016 - 19:31:48 CEST

This archive was generated by hypermail 2.3.0 : Tue Jun 07 2016 - 19:36:14 CEST