bin/174411: pw(8) core dump

[Poul-Henning Kamp]

>Number:         174411
>Category:       bin
>Synopsis:       pw(8) core dump
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Dec 13 10:50:00 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Poul-Henning Kamp
>Release:        FreeBSD 10.0-CURRENT amd64
>Organization:
>Environment:

FreeBSD c9.freebsd.dk 10.0-CURRENT FreeBSD 10.0-CURRENT #0 r244088: Mon Dec 10 16:36:05 UTC 2012     root at c9.freebsd.dk:/usr/obj/freebsd/svn_src/head/sys/GENERIC  amd64


>Description:

	The pw(8) program coredumps on bad memory management

>How-To-Repeat:

	On a freshly installed -current, executing:

	/usr/sbin/pw useradd phk -u 488 -d /home/phk \
		-c "Poul-Henning Kamp" -G "wheel,operator,dialer" \
		-s /bin/csh -w none

	This coredumps in jemalloc, from the call in line 761 in
	src/usr.sbin/pw/pw_user.c:

		if (j == 0)
			grp->gr_mem = NULL;
	>>>>>	grp->gr_mem = reallocf(grp->gr_mem, sizeof(*grp->gr_mem) *
						    (j + 2));
		grp->gr_mem[j] = pwd->pw_name;

	Reading the getgrent(3) manual page, it is far from clear to me
	that there is any reason to assume that grp->gr_mem is a malloced
	pointer.

	On the other hand, it is not clear to me that getgrent() is what
	is being called in the first place.

	Notice also the missing error handling on reallocf() failure,
	something more helpful than a somewhat-NULL pointer deref
	coredump could be called for.

>Fix:

	Rather than reallocf() a dedicated malloc() + memcopy() seems
	called for.
>Release-Note:
>Audit-Trail:
>Unformatted: