From e24adec1203cd25423ab2835a5be4f6b828b72a5 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Thu, 17 Dec 2020 03:28:45 +0000 Subject: Remove client-side abstract socket support CVE-2020-25697 and the Flatpak documentation show that clients using abstract sockets without mutual authentication is unsafe. TRANS_ABSTRACT remains supported, but it is now a no-op on the client side. Abstract sockets are still supported for servers, as the X server authenticates the client via other methods. Signed-off-by: Demi Marie Obenour --- Xtransint.h | 4 +++- Xtranssock.c | 33 +++------------------------------ 2 files changed, 6 insertions(+), 31 deletions(-) diff --git a/Xtransint.h b/Xtransint.h index b8f3b76..a43f7f8 100644 --- a/Xtransint.h +++ b/Xtransint.h @@ -297,7 +297,9 @@ typedef struct _Xtransport_table { #define TRANS_DISABLED (1<<2) /* Don't open this one */ #define TRANS_NOLISTEN (1<<3) /* Don't listen on this one */ #define TRANS_NOUNLINK (1<<4) /* Don't unlink transport endpoints */ -#define TRANS_ABSTRACT (1<<5) /* Use abstract sockets if available */ +#define TRANS_ABSTRACT (1<<5) /* This previously meant that abstract sockets should be used available. For security + * reasons, this is now a no-op on the client side, but it is still supported for servers. + */ #define TRANS_NOXAUTH (1<<6) /* Don't verify authentication (because it's secure some other way at the OS layer) */ #define TRANS_RECEIVED (1<<7) /* The fd for this has already been opened by someone else. */ diff --git a/Xtranssock.c b/Xtranssock.c index 99c0f1f..9482ecf 100644 --- a/Xtranssock.c +++ b/Xtranssock.c @@ -141,7 +141,7 @@ from the copyright holders. /* others don't need this */ #define SocketInitOnce() /**/ -#ifdef linux +#ifdef __linux__ #define HAVE_ABSTRACT_SOCKETS #endif @@ -1839,12 +1839,6 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, struct sockaddr_un sockname; SOCKLEN_T namelen; - - int abstract = 0; -#ifdef HAVE_ABSTRACT_SOCKETS - abstract = ciptr->transptr->flags & TRANS_ABSTRACT; -#endif - prmsg (2,"SocketUNIXConnect(%d,%s,%s)\n", ciptr->fd, host, port); /* @@ -1880,7 +1874,7 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, sockname.sun_family = AF_UNIX; - if (set_sun_path(port, UNIX_PATH, sockname.sun_path, abstract) != 0) { + if (set_sun_path(port, UNIX_PATH, sockname.sun_path, 0) != 0) { prmsg (1, "SocketUNIXConnect: path too long\n"); return TRANS_CONNECT_FAILED; } @@ -1896,16 +1890,6 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, #endif - - /* - * Adjust the socket path if using abstract sockets. - * Done here because otherwise all the strlen() calls above would fail. - */ - - if (abstract) { - sockname.sun_path[0] = '\0'; - } - /* * Do the connect() */ @@ -1939,15 +1923,7 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, return TRANS_IN_PROGRESS; else if (olderrno == EINTR) return TRANS_TRY_CONNECT_AGAIN; - else if (olderrno == ENOENT || olderrno == ECONNREFUSED) { - /* If opening as abstract socket failed, try again normally */ - if (abstract) { - ciptr->transptr->flags &= ~(TRANS_ABSTRACT); - return TRANS_TRY_CONNECT_AGAIN; - } else { - return TRANS_CONNECT_FAILED; - } - } else { + else { prmsg (2,"SocketUNIXConnect: Can't connect: errno = %d\n", EGET()); @@ -1969,9 +1945,6 @@ TRANS(SocketUNIXConnect) (XtransConnInfo ciptr, return TRANS_CONNECT_FAILED; } - if (abstract) - sockname.sun_path[0] = '@'; - ciptr->family = AF_UNIX; ciptr->addrlen = namelen; ciptr->peeraddrlen = namelen; -- cgit v1.2.1