Bug 27757

Summary:

chapss(1) converts a large uid to a negative one

Product:

Base System

Reporter:

Yoshihiro Koya <Yoshihiro.Koya>

Component:

bin

Assignee:

Mike Barcroft <mike>

Status:

Closed FIXED

Severity:

Affects Only Me

Priority:

Normal

Version:

4.3-STABLE

Hardware:

Any

OS:

Any

Attachments:

Description	Flags
file.diff	none

Description Yoshihiro Koya 2001-05-30 00:10:02 UTC

	A wrong format specifier of snprintf used in sources of
	chpass(1) generate a negative uid as a string.

How-To-Repeat: 
# vipw
(add some user with arbitrary uid)
# chapss foo
(edit as follows, for example)

#Changing user database information for foo.
Login: foo
Password: *
Uid [#]: 4294967295
Gid [# or name]: 20
Change [month day year]:
Expire [month day year]:
Class:
Home directory: /home/foo
Shell: /bin/csh
Full Name: User &
Office Location:
Office Phone:
Home Phone:
Other information:

(quit the editor. Then you would have ...)
/etc/pw.CRUoUQ: 15 lines, 291 characters.
chpass: -1 > recommended max uid value (65535)
chpass: updating the database...
pwd_mkdb: -1 > recommended max uid value (65535)
chpass: done

Also, you would find the following entry in your /etc/master.passwd

foo:*:-1:20:User &:/home/foo:/bin/csh

Comment 1 Bruce Evans 2001-05-30 11:51:53 UTC

On Wed, 30 May 2001, Yoshihiro Koya wrote:

> >Description:
> 	A wrong format specifier of snprintf used in sources of
> 	chpass(1) generate a negative uid as a string.
> 
> Index: edit.c
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/chpass/edit.c,v
> retrieving revision 1.18
> diff -u -r1.18 edit.c
> --- edit.c	2000/09/06 18:16:46	1.18
> +++ edit.c	2001/05/29 21:53:59
> @@ -255,7 +255,7 @@
>  		pw->pw_gecos[len - 1] = '\0';
>  
>  	if (snprintf(buf, sizeof(buf),
> -	    "%s:%s:%d:%d:%s:%ld:%ld:%s:%s:%s",
> +	    "%s:%s:%u:%u:%s:%ld:%ld:%s:%s:%s",
>  	    pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_class,
>  	    pw->pw_change, pw->pw_expire, pw->pw_gecos, pw->pw_dir,
>  	    pw->pw_shell) >= sizeof(buf)) {

There are some other printf format errors here.  The default promotion of
uid_t and gid_t is assumed to be u_int.  This just happens to be corect
on all supported machines.  The default promotion of time_t is assumed to
be long.  This happens to be incorrect on all supported machines (but may
magically work).

Bruce

Comment 2 Mike Barcroft freebsd_committer

2001-07-18 16:27:40 UTC

Responsible Changed
From-To: freebsd-bugs->mike


I'll take a look at this PR.

Comment 3 Mike Barcroft freebsd_committer

2001-07-19 04:10:11 UTC

Bruce,
Would you mind reviewing the patch at the end of this message?  I
believe it addresses your concerns regarding the orginator's patch
in PR #27757.

Best regards,
Mike Barcroft

----------------------------------------------------------------------

Index: chpass/chpass.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/chpass/chpass.c,v
retrieving revision 1.17
diff -u -r1.17 chpass.c
--- chpass/chpass.c	2000/09/06 18:16:46	1.17
+++ chpass/chpass.c	2001/07/19 00:39:41
@@ -173,7 +173,8 @@
 #else
 		case 0:
 			if (!(pw = getpwuid(uid)))
-				errx(1, "unknown user: uid %u", uid);
+				errx(1, "unknown user: uid %lu",
+				    (unsigned long)uid);
 			break;
 		case 1:
 			if (!(pw = getpwnam(*argv)))
Index: chpass/edit.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/chpass/edit.c,v
retrieving revision 1.18
diff -u -r1.18 edit.c
--- chpass/edit.c	2000/09/06 18:16:46	1.18
+++ chpass/edit.c	2001/07/19 00:39:41
@@ -115,8 +115,9 @@
 #endif /* YP */
 		(void)fprintf(fp, "Login: %s\n", pw->pw_name);
 		(void)fprintf(fp, "Password: %s\n", pw->pw_passwd);
-		(void)fprintf(fp, "Uid [#]: %d\n", pw->pw_uid);
-		(void)fprintf(fp, "Gid [# or name]: %d\n", pw->pw_gid);
+		(void)fprintf(fp, "Uid [#]: %lu\n", (unsigned long)pw->pw_uid);
+		(void)fprintf(fp, "Gid [# or name]: %lu\n",
+		    (unsigned long)pw->pw_gid);
 		(void)fprintf(fp, "Change [month day year]: %s\n",
 		    ttoa(pw->pw_change));
 		(void)fprintf(fp, "Expire [month day year]: %s\n",
@@ -255,9 +256,10 @@
 		pw->pw_gecos[len - 1] = '\0';
 
 	if (snprintf(buf, sizeof(buf),
-	    "%s:%s:%d:%d:%s:%ld:%ld:%s:%s:%s",
-	    pw->pw_name, pw->pw_passwd, pw->pw_uid, pw->pw_gid, pw->pw_class,
-	    pw->pw_change, pw->pw_expire, pw->pw_gecos, pw->pw_dir,
+	    "%s:%s:%lu:%lu:%s:%ld:%ld:%s:%s:%s",
+	    pw->pw_name, pw->pw_passwd, (unsigned long)pw->pw_uid, 
+	    (unsigned long)pw->pw_gid, pw->pw_class, (long)pw->pw_change,
+	    (long)pw->pw_expire, pw->pw_gecos, pw->pw_dir,
 	    pw->pw_shell) >= sizeof(buf)) {
 		warnx("entries too long");
 		free(p);
Index: chpass/pw_copy.c
===================================================================
RCS file: /home/ncvs/src/usr.bin/chpass/pw_copy.c,v
retrieving revision 1.9
diff -u -r1.9 pw_copy.c
--- chpass/pw_copy.c	1999/09/06 17:30:02	1.9
+++ chpass/pw_copy.c	2001/07/19 00:39:41
@@ -64,8 +64,8 @@
 	char chgstr[20];
 	char expstr[20];
 
-	snprintf(uidstr, sizeof(uidstr), "%d", pw->pw_uid);
-	snprintf(gidstr, sizeof(gidstr), "%d", pw->pw_gid);
+	snprintf(uidstr, sizeof(uidstr), "%lu", (unsigned long)pw->pw_uid);
+	snprintf(gidstr, sizeof(gidstr), "%lu", (unsigned long)pw->pw_gid);
 	snprintf(chgstr, sizeof(chgstr), "%ld", (long)pw->pw_change);
 	snprintf(expstr, sizeof(expstr), "%ld", (long)pw->pw_expire);

Comment 4 Mike Barcroft freebsd_committer

2001-08-02 15:39:14 UTC

State Changed
From-To: open->closed


This has been fixed in -CURRENT and -STABLE.