- dtucker@cvs.openbsd.org 2012/01/18 21:46:43

     [clientloop.c]
     Ensure that $DISPLAY contains only valid characters before using it to
     extract xauth data so that it can't be used to play local shell
     metacharacter games.  Report from r00t_ati at ihteam.net, ok markus.

2 files changed, 26 insertions, 1 deletions

diff --git a/ChangeLog b/ChangeLog
index 3ebe0df3..8eebcaff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -15,6 +15,11 @@
      Fix a memory leak in pkcs11_rsa_private_encrypt(), reported by Jan Klemkow.
      While there, be sure to buffer_clear() between send_msg() and recv_msg().
      ok markus@
+   - dtucker@cvs.openbsd.org 2012/01/18 21:46:43
+     [clientloop.c]
+     Ensure that $DISPLAY contains only valid characters before using it to
+     extract xauth data so that it can't be used to play local shell
+     metacharacter games.  Report from r00t_ati at ihteam.net, ok markus.
 
 20120206
  - (djm) [ssh-keygen.c] Don't fail in do_gen_all_hostkeys on platforms
diff --git a/clientloop.c b/clientloop.c
index 1339521f..f69a9b02 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.237 2011/09/10 22:26:34 markus Exp $ */
+/* $OpenBSD: clientloop.c,v 1.238 2012/01/18 21:46:43 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -281,6 +281,23 @@ set_control_persist_exit_time(void)
 	/* else we are already counting down to the timeout */
 }
 
+#define SSH_X11_VALID_DISPLAY_CHARS ":/.-_"
+static int
+client_x11_display_valid(const char *display)
+{
+	size_t i, dlen;
+
+	dlen = strlen(display);
+	for (i = 0; i < dlen; i++) {
+		if (!isalnum(display[i]) &&
+		    strchr(SSH_X11_VALID_DISPLAY_CHARS, display[i]) == NULL) {
+			debug("Invalid character '%c' in DISPLAY", display[i]);
+			return 0;
+		}
+	}
+	return 1;
+}
+
 #define SSH_X11_PROTO "MIT-MAGIC-COOKIE-1"
 void
 client_x11_get_proto(const char *display, const char *xauth_path,
@@ -303,6 +320,9 @@ client_x11_get_proto(const char *display, const char *xauth_path,
 
 	if (xauth_path == NULL ||(stat(xauth_path, &st) == -1)) {
 		debug("No xauth program.");
+	} else if (!client_x11_display_valid(display)) {
+		logit("DISPLAY '%s' invalid, falling back to fake xauth data",
+		    display);
 	} else {
 		if (display == NULL) {
 			debug("x11_get_proto: DISPLAY not set");