[Fixes] Buffer overruns due to overlenghted environment variables

Marc Lehmann schmorp at schmorp.de
Tue Aug 8 11:42:25 CEST 2006


On Tue, Aug 08, 2006 at 11:54:11AM +0200, Roland Baer <roland at verifysoft.de> wrote:
> Coverity Prevent hinted me to the unchecked "strcpy (sa.sun_path, sockname)"
> call in rxvtd.C:85. As we have to consistent with other uses of sockname,

Who is "we", and when you meant to write "we have to be", then why do we
have to be? Note that all uses within urxvt are consistent with each other
already.

> decided to fix that in rxvtdaemon.C, please apply attached patch.

Unfortunately, the patch doesn't fix anything: with the overrun, it very
likely either worked or you got a segfault, with your patch, it silently
fails, which is much worse behaviour than before, IMnsHO.

A proper diagnostic would be a real improvement, though (wether it
segfaults later or not is of no real consequence).

Also, do not define SUN_PATH_SIZE, and especially not using
0-pointer-sizeof-tricks, which are not guarenteed to work (this is
C++). Checking against sizeof (sa.sun_path) is clearer and portable.

Last not least you destroy the environment string, which can result in
segfaults (and unexpected behaviour for the user) on its own.

If all that were fixed the patch could be applied.

-- 
                The choice of a
      -----==-     _GNU_
      ----==-- _       generation     Marc Lehmann
      ---==---(_)__  __ ____  __      pcg at goof.com
      --==---/ / _ \/ // /\ \/ /      http://schmorp.de/
      -=====/_/_//_/\_,_/ /_/\_\      XX11-RIPE




More information about the rxvt-unicode mailing list