[PATCH] socket: fix read of non-connected socket

[Jon Simons <jon@xxxxxxxxxxxxx>]

Hi,

Attached is a patch to short-circuit the POLLIN drain loop
in ssh_socket_pollcallback for the case that the socket at
hand is closed during processing.


-Jon

From 46d327707ab348d38239de888cf9a535af5e9293 Mon Sep 17 00:00:00 2001
From: Jon Simons <jon@xxxxxxxxxxxxx>
Date: Thu, 30 Jan 2014 18:30:41 -0800
Subject: [PATCH] socket: fix read of non-connected socket

Ensure to check whether the socket at hand is indeed still connected
throughout POLLIN processing in ssh_socket_pollcallback.

Before this change, the POLLIN block in ssh_socket_pollcallback is
predicated against the condition (s->state == SSH_SOCKET_CONNECTED).
Once entered, data from the socket is consumed through the data
callback in this loop:

  do {
    r = s->callbacks->data(buffer_get_rest(s->in_buffer),
                           buffer_get_rest_len(s->in_buffer),
                           s->callbacks->userdata);
    buffer_pass_bytes(s->in_buffer,r);
  } while (r > 0);

However, it is possible for the socket data callback to change the
state of the socket (closing it, for example).  Fix the loop to only
continue so long as the socket remains connected: this also entails
setting the ssh_socket state to SSH_SOCKET_CLOSED upon close.

The bug can be observed before the change by sending a bogus banner
to the server: 'echo -e "A\r\nB\r\n" | nc localhost 22'.  Each of
'A' and 'B' will be processed by 'callback_receive_banner', even
though the client socket is closed after rejection of 'A'.
---
 src/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/socket.c b/src/socket.c
index 1a0bdea..5fa9102 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -291,7 +291,7 @@ int ssh_socket_pollcallback(struct ssh_poll_handle_struct *p, socket_t fd, int r
 							buffer_get_rest_len(s->in_buffer),
 							s->callbacks->userdata);
 					buffer_pass_bytes(s->in_buffer,r);
-				} while (r > 0);
+				} while ((r > 0) && (s->state == SSH_SOCKET_CONNECTED));
 				/* p may have been freed, so don't use it
 				* anymore in this function */
 				p = NULL;
@@ -440,6 +440,8 @@ void ssh_socket_close(ssh_socket s){
     ssh_poll_free(s->poll_out);
     s->poll_out=NULL;
   }
+
+  s->state = SSH_SOCKET_CLOSED;
 }
 
 /**
-- 
1.8.4.21.g992c386