From bcf8604f30fc754e7ff3cdb870ee6c9766e3f58c Mon Sep 17 00:00:00 2001 From: Keith Packard <keithp@keithp.com> Date: Wed, 5 Feb 2020 21:59:35 -0800 Subject: [PATCH] Allow display names to be arbitrary paths [v2] This lets the local listening socket live in a protected directory where other users cannot spoof it. Signed-off-by: Keith Packard <keithp@keithp.com> ---- v2: Update comment in VerifyDisplayName --- os/connection.c | 13 ++++++------- os/utils.c | 12 +++++++++--- xkb/ddxLoad.c | 4 ++++ 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/os/connection.c b/os/connection.c index 9e8d47f6b0..231d57e793 100644 --- a/os/connection.c +++ b/os/connection.c @@ -220,12 +220,8 @@ NotifyParentProcess(void) } static Bool -TryCreateSocket(int num, int *partial) +TryCreateSocket(const char *port, int *partial) { - char port[20]; - - snprintf(port, sizeof(port), "%d", num); - return (_XSERVTransMakeAllCOTSServerListeners(port, partial, &ListenTransCount, &ListenTransConns) >= 0); @@ -249,7 +245,7 @@ CreateWellKnownSockets(void) ListenTransCount = 0; } else if ((displayfd < 0) || explicit_display) { - if (TryCreateSocket(atoi(display), &partial) && + if (TryCreateSocket(display, &partial) && ListenTransCount >= 1) if (!PartialNetwork && partial) FatalError ("Failed to establish all listening sockets"); @@ -257,7 +253,10 @@ CreateWellKnownSockets(void) else { /* -displayfd and no explicit display number */ Bool found = 0; for (i = 0; i < 65536 - X_TCP_PORT; i++) { - if (TryCreateSocket(i, &partial) && !partial) { + char port[20]; + + snprintf(port, sizeof(port), "%d", i); + if (TryCreateSocket(port, &partial) && !partial) { found = 1; break; } diff --git a/os/utils.c b/os/utils.c index 92a66e81a3..298035af9f 100644 --- a/os/utils.c +++ b/os/utils.c @@ -598,8 +598,12 @@ UseMsg(void) /* This function performs a rudimentary sanity check * on the display name passed in on the command-line, * since this string is used to generate filenames. - * It is especially important that the display name - * not contain a "/" and not start with a "-". + * The string must either be a filename starting with "/", + * or a sequence of 1-3 numbers separated by ".". + * + * It is especially important that the display name not + * start with "-", and if the display name does not start + * with "/", then it must not contain a "/" * --kvajk */ static int @@ -617,6 +621,8 @@ VerifyDisplayName(const char *d) return 0; /* could be confused for an option */ if (*d == '.') return 0; /* must not equal "." or ".." */ + if (*d == '/') + return 1; if (strchr(d, '/') != (char *) 0) return 0; /* very important!!! */ @@ -847,7 +853,7 @@ ProcessCommandLine(int argc, char *argv[]) #ifdef LOCK_SERVER else if (strcmp(argv[i], "-nolock") == 0) { #if !defined(WIN32) && !defined(__CYGWIN__) - if (getuid() != 0) + if (getuid() != 0 && PrivsElevated()) ErrorF ("Warning: the -nolock option can only be used by root\n"); else diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c index f9b7b06d90..aa641a0550 100644 --- a/xkb/ddxLoad.c +++ b/xkb/ddxLoad.c @@ -105,6 +105,7 @@ RunXkbComp(xkbcomp_buffer_callback callback, void *userdata) char *xkbbasedirflag = NULL; const char *xkbbindir = emptystring; const char *xkbbindirsep = emptystring; + int i; #ifdef WIN32 /* WIN32 has no popen. The input must be stored in a file which is @@ -116,6 +117,9 @@ RunXkbComp(xkbcomp_buffer_callback callback, void *userdata) #endif snprintf(keymap, sizeof(keymap), "server-%s", display); + for (i = 0; keymap[i] != '\0'; i++) + if (keymap[i] == '/') + keymap[i] = '_'; OutputDirectory(xkm_output_dir, sizeof(xkm_output_dir)); -- GitLab