[PATCH libXau] Avoid heap corruption when calling XauFileName from multiple threads.

[Mark Kettenis]

> From: =?utf-8?q?Rami=20Ylim=C3=A4ki?= <rami.ylimaki at vincit.fi>
> Date: Mon, 28 Mar 2011 13:45:15 +0300
> 
> An XCB test application will always crash because of heap corruption
> if it's running xcb_connect/xcb_disconnect continuously from multiple
> threads. The problem can also happen in real applications if
> XOpenDisplay and xcb_connect are called simultaneously.
> 
> This commit fixes only the heap corruption and sporadic crashes. It's
> still possible that XauFileName returns a badly formed filename string
> if called from multiple threads. For example, changing contents of
> HOME environment variable could make the returned string to be
> malformed. However, there shouldn't be crashes.
> 
> Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
> Signed-off-by: Erkki Seppälä <erkki.seppala at vincit.fi>
> ---
>  AuFileName.c |   34 +++++++++++++++++++++-------------
>  1 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/AuFileName.c b/AuFileName.c
> index b21b048..fdf03e0 100644
> --- a/AuFileName.c
> +++ b/AuFileName.c
> @@ -33,14 +33,14 @@ in this Software without prior written authorization from The Open Group.
>  #include <X11/Xauth.h>
>  #include <X11/Xos.h>
>  #include <stdlib.h>
> +#include <limits.h>
>  
>  char *
>  XauFileName (void)
>  {
>      const char *slashDotXauthority = "/.Xauthority";
>      char    *name;
> -    static char	*buf;
> -    static int	bsize;
> +    static char	buf[PATH_MAX] = {0};

Static variables are automatically initialized to 0.  Doing so
explicitly will increase the size of the binary, so it's better not to
do this.

> -    strcpy (buf, name);
> -    strcat (buf, slashDotXauthority + (name[1] == '\0' ? 1 : 0));

> +    memcpy (buf, name, size);
> +    strcpy (buf + size, slashDotXauthority + ((size <= 1) ? 1 : 0));

This really looks like an obfuscation to me.  Since you do check that
the buffer is large enough beforehands, the origional
strcpy()/strcat() combo should be fine.  Or if you're paranoid, you
could use strncpy()/strncat().