- 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 3ebe0df30..8eebcaffb 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 1339521f4..f69a9b025 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");