Theo de Raadt wrote:
> > Some isfoo(char) usages crept back into ftp
>
> Hmm. I wonder how we can keep these errors out of base.
> Having to re-audit all the time is painful.
FWIW, this is a perfect use case for Coccinelle. Below is what I dredged
up in src/usr.sbin (diff not yet carefully audited, but apparently
sane).
I've been considering cronjobbing Coccinelle scripts (having the diffs
emailed to me) to prevent these sorts of entropic regressions.
--- httpd/httpd.c
+++ /tmp/cocci-output-27324-3d6efb-httpd.c
@@ -602,7 +602,7 @@ url_decode(char *url)
switch (*p) {
case '%':
/* Encoding character is followed by two hex chars */
- if (!(isxdigit(p[1]) && isxdigit(p[2])))
+ if (!(isxdigit((unsigned char)p[1]) &&
isxdigit((unsigned char)p[2])))
return (NULL);
hex[0] = p[1];
--- httpd/server_http.c
+++ /tmp/cocci-output-27324-ad673b-server_http.c
@@ -918,7 +918,7 @@ server_expand_http(struct client *clt, c
/* Find previously matched substrings by index */
for (p = val; clt->clt_srv_match.sm_nmatch &&
(p = strstr(p, "%")) != NULL; p++) {
- if (!isdigit(*(p + 1)))
+ if (!isdigit((unsigned char)*(p + 1)))
continue;
/* Copy number, leading '%' char and add trailing \0 */
--- ldapd/syntax.c
+++ /tmp/cocci-output-29625-30ee70-syntax.c
@@ -142,7 +142,7 @@ syntax_is_printable_string(struct schema
char *p;
for (p = value; len > 0 && *p != '\0'; p++, len--) {
- if (!isalnum(*p) && strchr(special, *p) == NULL)
+ if (!isalnum((unsigned char)*p) && strchr(special, *p) == NULL)
return 0;
}
--- smtpd/rfc2822.c
+++ /tmp/cocci-output-29655-69b554-rfc2822.c
@@ -93,13 +93,13 @@ parser_feed_header(struct rfc2822_parser
char *pos;
/* new header */
- if (! isspace(*line) && *line != '\0') {
+ if (! isspace((unsigned char)*line) && *line != '\0') {
rp->in_hdr = 1;
if ((pos = strchr(line, ':')) == NULL)
return 0;
memset(rp->header.name, 0, sizeof rp->header.name);
(void)memcpy(rp->header.name, line, pos - line);
- if (isspace(*(pos + 1)))
+ if (isspace((unsigned char)*(pos + 1)))
return parser_feed_header(rp, pos + 1);
else {
*pos = ' ';
@@ -169,7 +169,7 @@ rfc2822_parser_feed(struct rfc2822_parse
char buffer[RFC2822_MAX_LINE_SIZE+1];
/* in header and line is not a continuation, execute callback */
- if (rp->in_hdr && (*line == '\0' || !isspace(*line)))
+ if (rp->in_hdr && (*line == '\0' || !isspace((unsigned char)*line)))
header_callback(rp);
/* no longer in headers */
--- syslogd/syslogd.c
+++ /tmp/cocci-output-17550-6b6404-syslogd.c
@@ -1126,7 +1126,7 @@ octet_counting(struct evbuffer *evbuf, c
* It can be assumed that octet-counting framing is used if a syslog
* frame starts with a digit.
*/
- if (buf >= end || !isdigit(*buf))
+ if (buf >= end || !isdigit((unsigned char)*buf))
return (-1);
/*
* SYSLOG-FRAME = MSG-LEN SP SYSLOG-MSG
@@ -1134,7 +1134,7 @@ octet_counting(struct evbuffer *evbuf, c
* We support up to 5 digits in MSG-LEN, so the maximum is 99999.
*/
for (p = buf; p < end && p < buf + 5; p++) {
- if (!isdigit(*p))
+ if (!isdigit((unsigned char)*p))
break;
}
if (buf >= p || p >= end || *p != ' ')
--- tcpdump/print-decnet.c
+++ /tmp/cocci-output-17550-bc5c0e-print-decnet.c
@@ -761,7 +761,7 @@ pdata(u_char *dp, u_int maxlen)
while (x-- > 0) {
c = *dp++;
- if (isprint(c))
+ if (isprint((unsigned char)c))
putchar(c);
else
printf("\\%o", c & 0xFF);
--- tcpdump/print-ipsec.c
+++ /tmp/cocci-output-17550-499a71-print-ipsec.c
@@ -101,7 +101,7 @@ esp_init (char *espspec)
s[0] = espkey[2*i];
s[1] = espkey[2*i + 1];
s[2] = 0;
- if (!isxdigit(s[0]) || !isxdigit(s[1])) {
+ if (!isxdigit((unsigned char)s[0]) || !isxdigit((unsigned
char)s[1])) {
free(key);
error("espkey must be specified in hex");
}
--- tcpdump/smbutil.c
+++ /tmp/cocci-output-10974-0cf207-smbutil.c
@@ -148,7 +148,7 @@ static int name_interpret(const uchar *i
out--;
*out = '\0';
for (; *ob; ob++)
- if (!isprint(*ob))
+ if (!isprint((unsigned char)*ob))
*ob = 'X';
return(ret);
@@ -335,7 +335,7 @@ static const uchar *fdata1(const uchar *
int l = atoi(fmt+1);
buf += l;
fmt++;
- while (isdigit(*fmt)) fmt++;
+ while (isdigit((unsigned char)*fmt)) fmt++;
break;
}
case 'r':
@@ -425,14 +425,14 @@ static const uchar *fdata1(const uchar *
int l = atoi(fmt+1);
printf("%-*.*s",l,l,buf);
buf += l;
- fmt++; while (isdigit(*fmt)) fmt++;
+ fmt++; while (isdigit((unsigned char)*fmt)) fmt++;
break;
}
case 'h':
{
int l = atoi(fmt+1);
while (l--) printf("%02x",*buf++);
- fmt++; while (isdigit(*fmt)) fmt++;
+ fmt++; while (isdigit((unsigned char)*fmt)) fmt++;
break;
}
case 'n':
@@ -461,7 +461,7 @@ static const uchar *fdata1(const uchar *
buf += 16;
break;
}
- fmt++; while (isdigit(*fmt)) fmt++;
+ fmt++; while (isdigit((unsigned char)*fmt)) fmt++;
break;
}
case 'T':
@@ -491,7 +491,7 @@ static const uchar *fdata1(const uchar *
error("fdata1: invalid fmt: %s", fmt);
}
printf("%s",t?asctime(localtime(&t)):"NULL ");
- fmt++; while (isdigit(*fmt)) fmt++;
+ fmt++; while (isdigit((unsigned char)*fmt)) fmt++;
break;
}
default:
--- ypserv/stdethers/stdethers.c
+++ /tmp/cocci-output-2270-d04742-stdethers.c
@@ -148,9 +148,9 @@ main(int argc, char *argv[])
p = (char *) &data_line;
k = p; /* save start of key */
- while (!isspace(*p)) /* find first "space" */
+ while (!isspace((unsigned char)*p)) /* find first
"space" */
p++;
- while (isspace(*p)) /* move over "space" */
+ while (isspace((unsigned char)*p)) /* move over
"space" */
p++;
v = p; /* save start of value */
--- ypserv/stdhosts/stdhosts.c
+++ /tmp/cocci-output-2270-72dd65-stdhosts.c
@@ -112,9 +112,9 @@ main(int argc, char *argv[])
p = (char *) &data_line;
k = p; /* save start of key */
- while (!isspace(*p)) /* find first "space" */
+ while (!isspace((unsigned char)*p)) /* find first "space" */
p++;
- while (isspace(*p)) /* replace space with <NUL> */
+ while (isspace((unsigned char)*p)) /* replace space with
<NUL> */
*p++ = '\0';
v = p; /* save start of value */
--- ypserv/ypserv/acl.c
+++ /tmp/cocci-output-2270-df68c1-acl.c
@@ -161,7 +161,7 @@ acl_init(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -190,7 +190,7 @@ acl_init(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -231,7 +231,7 @@ acl_init(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -302,7 +302,7 @@ acl_init(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -325,7 +325,7 @@ acl_init(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -451,7 +451,7 @@ acl_securenet(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
@@ -481,7 +481,7 @@ acl_securenet(char *file)
k = p; /* save start of verb */
i = 0;
while (*p != '\0' &&
- !isspace((*p = tolower(*p)))) {
+ !isspace((unsigned char)(*p = tolower(*p)))) {
p++;
i++;
}
