[Xcb] [PATCH libXau] Avoid heap corruption when calling XauFileName from multiple threads.
Mark Kettenis
mark.kettenis at xs4all.nl
Mon Mar 28 04:59:25 PDT 2011
> 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().
More information about the Xcb
mailing list