user/1813: ssh/cli_read() fails to catch SIGINT + buffer overflow

[obdb@zzlevo.net]

>Number:         1813
>Category:       user
>Synopsis:       ssh/cli_read() fails to catch SIGINT + buffer overflow
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    bugs
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May  6 11:40:01 MDT 2001
>Last-Modified:
>Originator:     Andreas Gunnarsson
>Organization:
net
>Release:        -current
>Environment:
	
	System      : OpenBSD 2.9
	Architecture: OpenBSD.i386
	Machine     : i386
>Description:
	When SIGINT is caught while using cli_read() to read input, the
	variable intr is set to 1 by the signal handler. The check for
	intr should be performed before the check for errno == EINTR.

	The way it is now the following can happen:
	scp forks and starts ssh
	ssh asks for passphrase
	^C is pressed
	scp exits but ssh lingers
	when ssh finally exits it changes the terminal settings which can
	be bad if e.g. scp was called from an editor to remotely edit a
	file, or if the user has started entering another password.

	While writing this I just realized that this code also has a
	potential one character buffer overflow.
	When i is incremented in cli_read() it can become as large as size,
	which means that "buf[i] = '\0'" outside the loop can write outside
	the buffer.
>How-To-Repeat:
	scp remote_host:/blahblah
	^C when asked for passphrase
	ps | grep scp
>Fix:

Index: cli.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/cli.c,v
retrieving revision 1.11
diff -u -r1.11 cli.c
--- cli.c	2001/03/06 00:33:04	1.11
+++ cli.c	2001/05/06 17:04:17
@@ -143,15 +143,19 @@
 
Index: cli.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/cli.c,v
retrieving revision 1.11
diff -u -r1.11 cli.c
--- cli.c	2001/03/06 00:33:04	1.11
+++ cli.c	2001/05/06 17:20:48
@@ -143,15 +143,19 @@
 
 	while (ch != '\n') {
 		n = read(cli_input, &ch, 1);
+		if (intr)
+			break;
 		if (n == -1 && (errno == EAGAIN || errno == EINTR))
 			continue;
 		if (n != 1)
 			break;
-		if (ch == '\n' || intr != 0)
+		if (ch == '\n')
 			break;
-		if (i < size)
+		if (i < size - 1)
 			buf[i++] = ch;
 	}
+	if (intr)
+		i = 0;
 	buf[i] = '\0';
 
 	if (!echo)

>Audit-Trail:
>Unformatted: