OpenSSH -current fails regression on Solaris 8, sshd dumps core

[Darren Tucker]

Hi All.

While working on something I noticed a regression failure on Solaris 8.
It turned out to be present in -cvs and wasn't due to my changes.

One of the tests that fail is basically:
ssh -2 -F $build/regress/ssh_proxy 999.999.999.999 true

The server reports:
sshd[20529]: Disconnecting: Command terminated on signal 11.

The culprit seems to be session.c line 1019 or so:

  snprintf(buf, sizeof buf, "%.50s %d %.50s %d",
    get_remote_ipaddr(), get_remote_port(),
    get_local_ipaddr(packet_get_connection_in()), get_local_port());

After poking around, it seems that:
1) get_local_ipaddr returns NULL
2) this NULL is passed to snprintf
3) which dereferences the NULL causing a SEGV

(get_local_ipaddr returns NULL because it calls get_socket_address which
calls getpeername on a non-socket.)

The NULL doesn't seem to bother snprintf on Linux or HP-UX. I don't know
if it's valid to pass a NULL as an argument to "%s".

The attached patch fixes this problem but introduces more inconsistency
into the get_[local|remote|peer]_[ipaddr|name] functions in canohost.c.
There's probably a neater way of doing this.

The patch has been regression tested on Solaris 8, HP-UX 11 & Redhat
7.3.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
-------------- next part --------------
Index: canohost.c
===================================================================
RCS file: /cvs/openssh/canohost.c,v
retrieving revision 1.30
diff -u -r1.30 canohost.c
--- canohost.c	11 Jul 2002 03:56:47 -0000	1.30
+++ canohost.c	21 Sep 2002 14:28:42 -0000
@@ -246,10 +246,29 @@
 	return get_socket_address(socket, 1, NI_NUMERICHOST);
 }
 
-char *
-get_local_ipaddr(int socket)
+/*
+ * Returns the IP-address of the local host as a string.  The returned
+ * string must not be freed.
+ */
+
+const char *
+get_local_ipaddr(void)
 {
-	return get_socket_address(socket, 0, NI_NUMERICHOST);
+	static char *canonical_host_ip = NULL;
+
+	/* Check whether we have cached the ipaddr. */
+	if (canonical_host_ip == NULL) {
+		if (packet_connection_is_on_socket()) {
+			canonical_host_ip =
+			    get_socket_address(packet_get_connection_in(), 0, NI_NUMERICHOST);
+			if (canonical_host_ip == NULL)
+				fatal_cleanup();
+		} else {
+			/* If not on socket, return UNKNOWN. */
+			canonical_host_ip = xstrdup("UNKNOWN");
+		}
+	}
+	return canonical_host_ip;
 }
 
 char *
Index: canohost.h
===================================================================
RCS file: /cvs/openssh/canohost.h,v
retrieving revision 1.8
diff -u -r1.8 canohost.h
--- canohost.h	4 Jul 2001 04:46:57 -0000	1.8
+++ canohost.h	21 Sep 2002 14:28:42 -0000
@@ -18,7 +18,7 @@
 
 char		*get_peer_ipaddr(int);
 int		 get_peer_port(int);
-char		*get_local_ipaddr(int);
+const char	*get_local_ipaddr(void);
 char		*get_local_name(int);
 
 int		 get_remote_port(void);
Index: session.c
===================================================================
RCS file: /cvs/openssh/session.c,v
retrieving revision 1.220
diff -u -r1.220 session.c
--- session.c	19 Sep 2002 01:50:49 -0000	1.220
+++ session.c	21 Sep 2002 14:28:43 -0000
@@ -1018,7 +1018,7 @@
 
 	snprintf(buf, sizeof buf, "%.50s %d %.50s %d",
 	    get_remote_ipaddr(), get_remote_port(),
-	    get_local_ipaddr(packet_get_connection_in()), get_local_port());
+	    get_local_ipaddr(), get_local_port());
 	child_set_env(&env, &envsize, "SSH_CONNECTION", buf);
 
 	if (s->ttyfd != -1)