diff svr-x11fwd.c @ 1256:506f7681d0f8 coverity

merge up to date
author Matt Johnston <matt@ucc.asn.au>
date Tue, 15 Mar 2016 22:45:43 +0800
parents 428d83f2e5db
children 3a383aaeb487
line wrap: on
line diff
--- a/svr-x11fwd.c	Tue Dec 15 22:24:34 2015 +0800
+++ b/svr-x11fwd.c	Tue Mar 15 22:45:43 2016 +0800
@@ -42,11 +42,29 @@
 static int bindport(int fd);
 static int send_msg_channel_open_x11(int fd, struct sockaddr_in* addr);
 
+/* Check untrusted xauth strings for metacharacters */
+/* Returns DROPBEAR_SUCCESS/DROPBEAR_FAILURE */
+static int
+xauth_valid_string(const char *s)
+{
+	size_t i;
+
+	for (i = 0; s[i] != '\0'; i++) {
+		if (!isalnum(s[i]) &&
+		    s[i] != '.' && s[i] != ':' && s[i] != '/' &&
+		    s[i] != '-' && s[i] != '_') {
+			return DROPBEAR_FAILURE;
+		}
+	}
+	return DROPBEAR_SUCCESS;
+}
+
+
 /* called as a request for a session channel, sets up listening X11 */
 /* returns DROPBEAR_SUCCESS or DROPBEAR_FAILURE */
 int x11req(struct ChanSess * chansess) {
 
-	int fd;
+	int fd = -1;
 
 	if (!svr_pubkey_allows_x11fwd()) {
 		return DROPBEAR_FAILURE;
@@ -62,6 +80,11 @@
 	chansess->x11authcookie = buf_getstring(ses.payload, NULL);
 	chansess->x11screennum = buf_getint(ses.payload);
 
+	if (xauth_valid_string(chansess->x11authprot) == DROPBEAR_FAILURE ||
+		xauth_valid_string(chansess->x11authcookie) == DROPBEAR_FAILURE) {
+		dropbear_log(LOG_WARNING, "Bad xauth request");
+		goto fail;
+	}
 	/* create listening socket */
 	fd = socket(PF_INET, SOCK_STREAM, 0);
 	if (fd < 0) {
@@ -142,7 +165,7 @@
 	}
 
 	/* create the DISPLAY string */
-	val = snprintf(display, sizeof(display), "localhost:%d.%d",
+	val = snprintf(display, sizeof(display), "localhost:%d.%u",
 			chansess->x11port - X11BASEPORT, chansess->x11screennum);
 	if (val < 0 || val >= (int)sizeof(display)) {
 		/* string was truncated */
@@ -152,14 +175,14 @@
 	addnewvar("DISPLAY", display);
 
 	/* create the xauth string */
-	val = snprintf(display, sizeof(display), "unix:%d.%d",
+	val = snprintf(display, sizeof(display), "unix:%d.%u",
 			chansess->x11port - X11BASEPORT, chansess->x11screennum);
 	if (val < 0 || val >= (int)sizeof(display)) {
 		/* string was truncated */
 		return;
 	}
 
-	/* popen is a nice function - code is strongly based on OpenSSH's */
+	/* code is strongly based on OpenSSH's */
 	authprog = popen(XAUTH_COMMAND, "w");
 	if (authprog) {
 		fprintf(authprog, "add %s %s %s\n",