Call for testing: OpenSSH-5.6

Alan Barrett apb at cequrux.com
Thu Aug 12 08:22:30 EST 2010


On Tue, 10 Aug 2010, Damien Miller wrote:
> OpenSSH 5.6 is almost ready for release, so we would appreciate testing
> on as many platforms and systems as possible. This is a moderately large
> release, with a number of new features and bug fixes.

I attach two patches relative to the 20100810 snapshot.

1) ctype(3) functions should not be called with char arguments;
   they should be called with unsigned char arguments.
2) in sftp-common.c, strmode() requires #include <unistd.h>,
   and the results of user_from_uid() and group_from_gid()
   are pointers to const char, not pointers to plain char

I also attach the semantic patch that generated patch 1.  Use
"spatch -inplace -sp_file ctype.spatch -dir .".

The result compiles successfully with "-Werror" appended to CFLAGS,
and passes all tests on NetBSD-5.99.27/i386.

--apb (Alan Barrett)
-------------- next part --------------
diff --git a/canohost.c b/canohost.c
index ef94d91..8cf7005 100644
--- a/canohost.c
+++ b/canohost.c
@@ -104,8 +104,8 @@ get_remote_hostname(int sock, int use_dns)
 	 * of this software).
 	 */
 	for (i = 0; name[i]; i++)
-		if (isupper(name[i]))
-			name[i] = (char)tolower(name[i]);
+		if (isupper((unsigned char)name[i]))
+			name[i] = (char)tolower((unsigned char)name[i]);
 	/*
 	 * Map it back to an IP address and check that the given
 	 * address actually is an address of this host.  This is
diff --git a/clientloop.c b/clientloop.c
index de79793..10bda5f 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -831,7 +831,7 @@ process_cmdline(void)
 	cmd = s = read_passphrase("\r\nssh> ", RP_ECHO);
 	if (s == NULL)
 		goto out;
-	while (isspace(*s))
+	while (isspace((unsigned char)*s))
 		s++;
 	if (*s == '-')
 		s++;	/* Skip cmdline '-', if any */
@@ -885,7 +885,7 @@ process_cmdline(void)
 		goto out;
 	}
 
-	while (isspace(*++s))
+	while (isspace((unsigned char)*++s))
 		;
 
 	/* XXX update list of forwards in options */
diff --git a/match.c b/match.c
index 2389477..d36134c 100644
--- a/match.c
+++ b/match.c
@@ -140,8 +140,8 @@ match_pattern_list(const char *string, const char *pattern, u_int len,
 		for (subi = 0;
 		    i < len && subi < sizeof(sub) - 1 && pattern[i] != ',';
 		    subi++, i++)
-			sub[subi] = dolower && isupper(pattern[i]) ?
-			    (char)tolower(pattern[i]) : pattern[i];
+			sub[subi] = dolower && isupper((unsigned char)pattern[i]) ?
+			    (char)tolower((unsigned char)pattern[i]) : pattern[i];
 		/* If subpattern too long, return failure (no match). */
 		if (subi >= sizeof(sub) - 1)
 			return 0;
diff --git a/openbsd-compat/fmt_scaled.c b/openbsd-compat/fmt_scaled.c
index edd682a..709db6f 100644
--- a/openbsd-compat/fmt_scaled.c
+++ b/openbsd-compat/fmt_scaled.c
@@ -81,7 +81,7 @@ scan_scaled(char *scaled, long long *result)
 	long long scale_fact = 1, whole = 0, fpart = 0;
 
 	/* Skip leading whitespace */
-	while (isascii(*p) && isspace(*p))
+	while (isascii(*p) && isspace((unsigned char)*p))
 		++p;
 
 	/* Then at most one leading + or - */
@@ -108,7 +108,7 @@ scan_scaled(char *scaled, long long *result)
 	 * (but note that E for Exa might look like e to some!).
 	 * Advance 'p' to end, to get scale factor.
 	 */
-	for (; isascii(*p) && (isdigit(*p) || *p=='.'); ++p) {
+	for (; isascii(*p) && (isdigit((unsigned char)*p) || *p=='.'); ++p) {
 		if (*p == '.') {
 			if (fract_digits > 0) {	/* oops, more than one '.' */
 				errno = EINVAL;
@@ -152,10 +152,10 @@ scan_scaled(char *scaled, long long *result)
 
 		/** Are we there yet? */
 		if (*p == scale_chars[i] ||
-			*p == tolower(scale_chars[i])) {
+			*p == tolower((unsigned char)scale_chars[i])) {
 
 			/* If it ends with alphanumerics after the scale char, bad. */
-			if (isalnum(*(p+1))) {
+			if (isalnum((unsigned char)*(p + 1))) {
 				errno = EINVAL;
 				return -1;
 			}
diff --git a/openbsd-compat/inet_aton.c b/openbsd-compat/inet_aton.c
index 130597e..7247c8b 100644
--- a/openbsd-compat/inet_aton.c
+++ b/openbsd-compat/inet_aton.c
@@ -100,7 +100,7 @@ inet_aton(const char *cp, struct in_addr *addr)
 		 * Values are specified as for C:
 		 * 0x=hex, 0=octal, isdigit=decimal.
 		 */
-		if (!isdigit(c))
+		if (!isdigit((unsigned char)c))
 			return (0);
 		val = 0; base = 10;
 		if (c == '0') {
@@ -111,12 +111,12 @@ inet_aton(const char *cp, struct in_addr *addr)
 				base = 8;
 		}
 		for (;;) {
-			if (isascii(c) && isdigit(c)) {
+			if (isascii(c) && isdigit((unsigned char)c)) {
 				val = (val * base) + (c - '0');
 				c = *++cp;
-			} else if (base == 16 && isascii(c) && isxdigit(c)) {
+			} else if (base == 16 && isascii(c) && isxdigit((unsigned char)c)) {
 				val = (val << 4) |
-					(c + 10 - (islower(c) ? 'a' : 'A'));
+					(c + 10 - (islower((unsigned char)c) ? 'a' : 'A'));
 				c = *++cp;
 			} else
 				break;
@@ -138,7 +138,7 @@ inet_aton(const char *cp, struct in_addr *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
diff --git a/openbsd-compat/mktemp.c b/openbsd-compat/mktemp.c
index 2285c84..38f0153 100644
--- a/openbsd-compat/mktemp.c
+++ b/openbsd-compat/mktemp.c
@@ -159,7 +159,7 @@ _gettemp(path, doopen, domkdir, slen)
 					return (0);
 				*trv++ = 'a';
 			} else {
-				if (isdigit(*trv))
+				if (isdigit((unsigned char)*trv))
 					*trv = 'a';
 				else if (*trv == 'z')	/* inc from z to A */
 					*trv = 'A';
diff --git a/openbsd-compat/readpassphrase.c b/openbsd-compat/readpassphrase.c
index 62b6d0d..e4dd1c9 100644
--- a/openbsd-compat/readpassphrase.c
+++ b/openbsd-compat/readpassphrase.c
@@ -131,11 +131,11 @@ restart:
 			if (p < end) {
 				if ((flags & RPP_SEVENBIT))
 					ch &= 0x7f;
-				if (isalpha(ch)) {
+				if (isalpha((unsigned char)ch)) {
 					if ((flags & RPP_FORCELOWER))
-						ch = (char)tolower(ch);
+						ch = (char)tolower((unsigned char)ch);
 					if ((flags & RPP_FORCEUPPER))
-						ch = (char)toupper(ch);
+						ch = (char)toupper((unsigned char)ch);
 				}
 				*p++ = ch;
 			}
diff --git a/readconf.c b/readconf.c
index 0296590..3927204 100644
--- a/readconf.c
+++ b/readconf.c
@@ -539,7 +539,7 @@ parse_yesnoask:
 		orig = val64 = strtoll(arg, &endofnumber, 10);
 		if (arg == endofnumber)
 			fatal("%.200s line %d: Bad number.", filename, linenum);
-		switch (toupper(*endofnumber)) {
+		switch (toupper((unsigned char)*endofnumber)) {
 		case '\0':
 			scale = 1;
 			break;
@@ -1294,7 +1294,7 @@ parse_forward(Forward *fwd, const char *fwdspec, int dynamicfwd, int remotefwd)
 	cp = p = xstrdup(fwdspec);
 
 	/* skip leading spaces */
-	while (isspace(*cp))
+	while (isspace((unsigned char)*cp))
 		cp++;
 
 	for (i = 0; i < 4; ++i)
diff --git a/scp.c b/scp.c
index e07de42..57c1490 100644
--- a/scp.c
+++ b/scp.c
@@ -988,7 +988,7 @@ sink(int argc, char **argv)
 		if (*cp++ != ' ')
 			SCREWUP("mode not delimited");
 
-		for (size = 0; isdigit(*cp);)
+		for (size = 0; isdigit((unsigned char)*cp);)
 			size = size * 10 + (*cp++ - '0');
 		if (*cp++ != ' ')
 			SCREWUP("size not delimited");
diff --git a/sftp.c b/sftp.c
index 229f129..ff0cb7f 100644
--- a/sftp.c
+++ b/sftp.c
@@ -986,7 +986,7 @@ makeargv(const char *arg, int *argcp, int sloppy, char *lastquote,
 	state = MA_START;
 	i = j = 0;
 	for (;;) {
-		if (isspace(arg[i])) {
+		if (isspace((unsigned char)arg[i])) {
 			if (state == MA_UNQUOTED) {
 				/* Terminate current argument */
 				argvs[j++] = '\0';
diff --git a/ssh.c b/ssh.c
index ab37c20..60b2284 100644
--- a/ssh.c
+++ b/ssh.c
@@ -711,8 +711,8 @@ main(int ac, char **av)
 	/* force lowercase for hostkey matching */
 	if (options.host_key_alias != NULL) {
 		for (p = options.host_key_alias; *p; p++)
-			if (isupper(*p))
-				*p = (char)tolower(*p);
+			if (isupper((unsigned char)*p))
+				*p = (char)tolower((unsigned char)*p);
 	}
 
 	if (options.proxy_command != NULL &&
diff --git a/sshconnect.c b/sshconnect.c
index f55beff..7d1110f 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -1106,8 +1106,8 @@ ssh_login(Sensitive *sensitive, const char *orighost,
 	/* Convert the user-supplied hostname into all lowercase. */
 	host = xstrdup(orighost);
 	for (cp = host; *cp; cp++)
-		if (isupper(*cp))
-			*cp = (char)tolower(*cp);
+		if (isupper((unsigned char)*cp))
+			*cp = (char)tolower((unsigned char)*cp);
 
 	/* Exchange protocol version identification strings with the server. */
 	ssh_exchange_identification(timeout_ms);
-------------- next part --------------
diff --git a/sftp-common.c b/sftp-common.c
index a042875..d0d7de7 100644
--- a/sftp-common.c
+++ b/sftp-common.c
@@ -36,6 +36,7 @@
 #include <string.h>
 #include <time.h>
 #include <stdarg.h>
+#include <unistd.h>
 #ifdef HAVE_UTIL_H
 #include <util.h>
 #endif
@@ -191,7 +192,7 @@ ls_file(const char *name, const struct stat *st, int remote, int si_units)
 {
 	int ulen, glen, sz = 0;
 	struct tm *ltime = localtime(&st->st_mtime);
-	char *user, *group;
+	const char *user, *group;
 	char buf[1024], mode[11+1], tbuf[12+1], ubuf[11+1], gbuf[11+1];
 	char sbuf[FMT_SCALED_STRSIZE];
 
-------------- next part --------------
// cast to (unsigned char) where ctype(3) functions are called
// with char args.
//
// Note that isascii is not included in the list of functions to modify,
// because it is well-defined on the entire range of integers.
// 

@ bad_ctype expression @
char X;
@@
(
 isalpha
|
 isupper
|
 islower
|
 isdigit
|
 isxdigit
|
 isalnum
|
 isspace
|
 isalnum
|
 isspace
|
 ispunct
|
 isprint
|
 isgraph
|
 iscntrl
|
 isblank
|
 toupper
|
 tolower
)
-(X)
+((unsigned char)X)


More information about the openssh-unix-dev mailing list