Paul Eggert
2001-Sep-28 19:01 UTC
openssh-2.9.9p2 subscript violation problems with ctype macros
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();
Seemingly Similar Threads
- [Bug 377] New: Reduce compiler warnings. Use unsigned args to the ctype.h is*() macros.
- openssh-2.9.9p2 assumes pid_t, uid_t, etc. are not 'long'
- openssh-2.9.9p2 session.c uses two undeclared void functions
- get rid of various warnings, errors in io.h
- Call for testing: OpenSSH-5.6