Subject: bin/20156: Wrong usage of is{digit,alnum,space} in user
To: None <gnats-bugs@gnats.netbsd.org>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 02/02/2003 00:37:37
>Number:         20156
>Category:       bin
>Synopsis:       Wrong usage of is{digit,alnum,space} in user
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 01 15:38:01 PST 2003
>Closed-Date:
>Last-Modified:
>Originator:     Christian Biere
>Release:        NetBSD 1.6K
>Organization:
>Environment:

>Description:
While reading PR misc/20154 I've noticed the following:

In src/usr.sbin/user/user.c isalnum(), isdigit() and isspace() are used
with a char as parameter. On platforms with char being signed char by
default this means calling those function with invalid arguments and
results are undefined.
Note, that those functions also respect the locale settings which might
not have been considered here. I haven't verified whether this is of any
concern.

>How-To-Repeat:


>Fix:

--Multipart_Sun__2_Feb_2003_00:37:37_+0100_082bda00
Content-Type: text/plain;
 name="user.c.udif"
Content-Disposition: attachment;
 filename="user.c.udif"
Content-Transfer-Encoding: 7bit

Index: user.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/user/user.c,v
retrieving revision 1.65
diff -u -r1.65 user.c
--- user.c	2002/11/08 11:53:20	1.65
+++ user.c	2003/02/01 23:20:56
@@ -354,7 +354,7 @@
 is_number(char *s)
 {
 	for ( ; *s ; s++) {
-		if (!isdigit(*s)) {
+		if (!isdigit((unsigned char) *s)) {
 			return 0;
 		}
 	}
@@ -636,7 +636,7 @@
 static int
 valid_login(char *login_name)
 {
-	char	*cp;
+	unsigned char	*cp;
 
 	for (cp = login_name ; *cp ; cp++) {
 		if (!isalnum(*cp) && *cp != '.' && *cp != '_' && *cp != '-') {
@@ -650,7 +650,7 @@
 static int
 valid_group(char *group)
 {
-	char	*cp;
+	unsigned char	*cp;
 
 	for (cp = group ; *cp ; cp++) {
 		if (!isalnum(*cp)) {
@@ -765,8 +765,8 @@
 	size_t		lineno;
 	size_t		len;
 	FILE		*fp;
-	char		*cp;
-	char		*s;
+	unsigned char	*cp;
+	unsigned char	*s;
 
 	memsave(&up->u_primgrp, DEF_GROUP, strlen(DEF_GROUP));
 	memsave(&up->u_basedir, DEF_BASEDIR, strlen(DEF_BASEDIR));
@@ -931,7 +931,7 @@
 			*tp = mktime(&tm);
 		} else if (strptime(s, "%B %d %Y", &tm) != NULL) {
 			*tp = mktime(&tm);
-		} else if (isdigit(s[0]) != NULL) {
+		} else if (isdigit((unsigned char) s[0]) != NULL) {
 			*tp = atoi(s);
 		} else {
 			return 0;

--Multipart_Sun__2_Feb_2003_00:37:37_+0100_082bda00--
>Release-Note:
>Audit-Trail:
>Unformatted:
 This is a multi-part message in MIME format.
 
 --Multipart_Sun__2_Feb_2003_00:37:37_+0100_082bda00
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit