Re: [musl] [PATCH] Zero the leading stack canary byte

[Rich Felker <dalias@libc.org>]

On Mon, Dec 13, 2021 at 01:24:38PM +0100, jvoisin wrote:
> > As written this patch assumes a 64-bit uintptr_t, which isn't ok.
> > Indeed 56 bits should be fine on a 64-bit arch, but dropping from 32
> > to 24 on a 32-bit arch severely weakens the protection. So it probably
> > needs to be conditional on 64-bit.
> Will do.
> 
> > Also, zeroing the first byte means we can no longer catch buffer
> > overflows of the form "off-by-one string length". This seems
> > unfortunate. Putting the 0 byte at the end would solve that at the
> > expense of allowing the canary value to be leaked via missing
> > termination bugs, and overall I would lean towards catching actual
> > buffer overflow bugs vs stopping canary leaks.
> As discussed on IRC, what about zero'ing the second byte instead? This
> would allow to catch overflows, as well as preventing canary
> leaks/overwrite via string-manipulating functions.

That seems like a good idea too. Note that it can be done without
masking logic and large constants just with:

((char *)&__stack_chk_guard)[1] = 0;

And beginning or end could have been done just by reducing the memcpy
length by 1.

Rich