openssh-2.9.9p2 subscript violation problems with ctype macros

Paul Eggert eggert at twinsun.com
Sat Sep 29 05:01:41 EST 2001


In several places, openssh-2.9.9p2 passes a 'char' value to a ctype
macro like 'isdigit'.  This has undefined behavior on hosts with
signed characters, if the character value happens to be negative.  For
example, isdigit('\200') expands to an array access that is a
subscript error on hosts with signed characters where '\200' == -128.
This leads to incorrect results, and possibly even core dumps.

The C standard says that isdigit(X) has undefined behavior unless
X == EOF || X == (unsigned char)X.

Here is a patch.  This patch occasionally uses '0'<=X && X<='9' rather
than isdigit(X), as it's faster on most modern hosts (a subtract and a
compare, rather than a reference through memory and then a test).

2001-09-28  Paul Eggert  <eggert at twinsun.com>

	* canohost.c (get_remote_hostname):
	Don't pass negative chars to ctype macros.
	* match.c (match_hostname): Likewise.
	* openbsd-compat/base64.c (b64_ntop): Likewise.
	* openbsd-compat/inet_aton.c (inet_aton): Likewise.
	* openbsd-compat/mktemp.c (_gettemp): Likewise.
	* openbsd-compat/readpassphrase.c (readpassphrase): Likewise.
	* scp.c (sink): Likewise.
	* sshconnect.c (sshconnect.c): Likewise.

===================================================================
RCS file: canohost.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- canohost.c	2001/06/25 05:01:24	2.9.9.2
+++ canohost.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -87,8 +87,8 @@ get_remote_hostname(int socket, int reve
 	 * of this software).
 	 */
 	for (i = 0; name[i]; i++)
-		if (isupper(name[i]))
-			name[i] = tolower(name[i]);
+		if (isupper((unsigned char)name[i]))
+			name[i] = tolower((unsigned char)name[i]);
 
 	if (!reverse_mapping_check)
 		return xstrdup(name);
===================================================================
RCS file: match.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- match.c	2001/07/04 04:56:46	2.9.9.2
+++ match.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -134,7 +134,7 @@ match_hostname(const char *host, const c
 		for (subi = 0;
 		     i < len && subi < sizeof(sub) - 1 && pattern[i] != ',';
 		     subi++, i++)
-			sub[subi] = isupper(pattern[i]) ? tolower(pattern[i]) : pattern[i];
+			sub[subi] = isupper((unsigned char)pattern[i]) ? tolower((unsigned char)pattern[i]) : pattern[i];
 		/* If subpattern too long, return failure (no match). */
 		if (subi >= sizeof(sub) - 1)
 			return 0;
===================================================================
RCS file: openbsd-compat/base64.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- openbsd-compat/base64.c	2001/01/31 21:52:03	2.9.9.2
+++ openbsd-compat/base64.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -199,7 +199,8 @@ b64_ntop(u_char const *src, size_t srcle
 int
 b64_pton(char const *src, u_char *target, size_t targsize)
 {
-	int tarindex, state, ch;
+	int tarindex, state;
+	unsigned char ch;
 	char *pos;
 
 	state = 0;
===================================================================
RCS file: openbsd-compat/inet_aton.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- openbsd-compat/inet_aton.c	2001/01/31 21:52:03	2.9.9.2
+++ openbsd-compat/inet_aton.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -114,7 +114,7 @@ inet_aton(const char *cp, struct in_addr
 		 * Values are specified as for C:
 		 * 0x=hex, 0=octal, isdigit=decimal.
 		 */
-		if (!isdigit(c))
+		if (! ('0' <= c && c <= '9'))
 			return (0);
 		val = 0; base = 10;
 		if (c == '0') {
@@ -125,12 +125,14 @@ inet_aton(const char *cp, struct in_addr
 				base = 8;
 		}
 		for (;;) {
-			if (isascii(c) && isdigit(c)) {
+			if ('0' <= c && c <= '9') {
 				val = (val * base) + (c - '0');
 				c = *++cp;
-			} else if (base == 16 && isascii(c) && isxdigit(c)) {
-				val = (val << 4) |
-					(c + 10 - (islower(c) ? 'a' : 'A'));
+			} else if (base == 16 && 'a' <= c && c <= 'f') {
+				val = (val << 4) | (c + 10 - 'a');
+				c = *++cp;
+			} else if (base == 16 && 'A' <= c && c <= 'F') {
+				val = (val << 4) | (c + 10 - 'A');
 				c = *++cp;
 			} else
 				break;
@@ -152,7 +154,7 @@ inet_aton(const char *cp, struct in_addr
 	/*
 	 * Check for trailing characters.
 	 */
-	if (c != '\0' && (!isascii(c) || !isspace(c)))
+	if (c != '\0' && (!isascii(c) || !isspace((unsigned char)c)))
 		return (0);
 	/*
 	 * Concoct the address according to
===================================================================
RCS file: openbsd-compat/mktemp.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- openbsd-compat/mktemp.c	2001/01/31 21:52:03	2.9.9.2
+++ openbsd-compat/mktemp.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -164,7 +164,7 @@ _gettemp(path, doopen, domkdir, slen)
 					return (0);
 				*trv++ = 'a';
 			} else {
-				if (isdigit(*trv))
+				if ('0' <= *trv && *trv <= '9')
 					*trv = 'a';
 				else if (*trv == 'z')	/* inc from z to A */
 					*trv = 'A';
===================================================================
RCS file: openbsd-compat/readpassphrase.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- openbsd-compat/readpassphrase.c	2001/06/29 12:35:13	2.9.9.2
+++ openbsd-compat/readpassphrase.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -118,11 +118,11 @@ readpassphrase(prompt, buf, bufsiz, flag
 		if (p < end) {
 			if ((flags & RPP_SEVENBIT))
 				ch &= 0x7f;
-			if (isalpha(ch)) {
+			if (isalpha((unsigned char)ch)) {
 				if ((flags & RPP_FORCELOWER))
-					ch = tolower(ch);
+					ch = tolower((unsigned char)ch);
 				if ((flags & RPP_FORCEUPPER))
-					ch = toupper(ch);
+					ch = toupper((unsigned char)ch);
 			}
 			*p++ = ch;
 		}
===================================================================
RCS file: scp.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- scp.c	2001/09/20 00:57:56	2.9.9.2
+++ scp.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -761,7 +761,7 @@ sink(argc, argv)
 		if (*cp++ != ' ')
 			SCREWUP("mode not delimited");
 
-		for (size = 0; isdigit(*cp);)
+		for (size = 0; '0' <= *cp && *cp <= '9';)
 			size = size * 10 + (*cp++ - '0');
 		if (*cp++ != ' ')
 			SCREWUP("size not delimited");
===================================================================
RCS file: sshconnect.c,v
retrieving revision 2.9.9.2
retrieving revision 2.9.9.2.0.1
diff -pu -r2.9.9.2 -r2.9.9.2.0.1
--- sshconnect.c	2001/08/07 22:29:09	2.9.9.2
+++ sshconnect.c	2001/09/28 18:48:11	2.9.9.2.0.1
@@ -881,8 +881,8 @@ ssh_login(Key **keys, int nkeys, const c
 	/* Convert the user-supplied hostname into all lowercase. */
 	host = xstrdup(orighost);
 	for (cp = host; *cp; cp++)
-		if (isupper(*cp))
-			*cp = tolower(*cp);
+		if (isupper((unsigned char)*cp))
+			*cp = tolower((unsigned char)*cp);
 
 	/* Exchange protocol version identification strings with the server. */
 	ssh_exchange_identification();



More information about the openssh-unix-dev mailing list