Bug 198554 - erroneous data in master.passwd or group cause pw -V command to segfault
Summary: erroneous data in master.passwd or group cause pw -V command to segfault
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.1-STABLE
Hardware: amd64 Any
: --- Affects Only Me
Assignee: Baptiste Daroussin
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-03-13 00:24 UTC by diaren
Modified: 2015-07-07 21:08 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description diaren 2015-03-13 00:24:18 UTC
When using `pw -V ...`, if either master.passwd or group contains erroneous data then pw will segfault. e.g. as a result of hand editing. Also could be a continuation of bug #187310.

To reproduce execute the following in an empty directory:

truncate -s 0 master.passwd group && pwd_mkdb -d . master.passwd
pw -V . user add test
echo 'test1:*:1002:0:0::/home/test1' >> master.passwd
pw -V . user add test2
Segmentation fault (core dumped)

Similarly for group:

truncate -s 0 master.passwd group && pwd_mkdb -d . master.passwd
pw -V . user add test
echo 'test1:*:1002' >> group
pw -V . user add test2
Segmentation fault (core dumped)

It doesn't matter what pw -V command  you run, if it accesses an erroneous master.passwd/group it segfaults.

I first came across it in:
FreeBSD 10.1-STABLE #0 r279301: Wed Feb 25 23:49:09 UTC 2015 amd64
and have since updated to r279937

The following fixes the segfaults, but could lead to other issues as the erroneous entries are simply ignored.

Index: usr.sbin/pw/pw_vpw.c
===================================================================
--- usr.sbin/pw/pw_vpw.c	(revision 279937)
+++ usr.sbin/pw/pw_vpw.c	(working copy)
@@ -80,6 +80,9 @@
 			if (line[linelen - 1 ] == '\n')
 				line[linelen - 1] = '\0';
 			pw = pw_scan(line, PWSCAN_MASTER);
+			/* Skip erroneous lines... maybe warn? */
+			if (pw == NULL)
+			    continue;
 			if (uid != (uid_t)-1) {
 				if (uid == pw->pw_uid)
 					break;
@@ -160,6 +163,9 @@
 			if (line[linelen - 1 ] == '\n')
 				line[linelen - 1] = '\0';
 			gr = gr_scan(line);
+			/* Skip erroneous lines.. maybe warn? */
+			if (gr == NULL)
+			    continue;
 			if (gid != (gid_t)-1) {
 				if (gid == gr->gr_gid)
 					break;
Comment 1 Glen Barber freebsd_committer freebsd_triage 2015-07-07 16:54:54 UTC
Baptiste, since you touched pw(8) last, would you mind looking at this?
Comment 2 commit-hook freebsd_committer freebsd_triage 2015-07-07 21:05:49 UTC
A commit references this bug:

Author: bapt
Date: Tue Jul  7 21:05:22 UTC 2015
New revision: 285256
URL: https://svnweb.freebsd.org/changeset/base/285256

Log:
  pw: fail if an invalid entry is found while parsing master.passwd and group

  PR:		198554
  Reported by:	diaran <fbsd@centraltech.co.uk>
  MFC after:	2 days

Changes:
  head/usr.sbin/pw/pw_vpw.c
  head/usr.sbin/pw/tests/pw_useradd.sh
Comment 3 Baptiste Daroussin freebsd_committer freebsd_triage 2015-07-07 21:08:17 UTC
I have preferred to fail rather than ignoring, IMHO such issue is important a deserves the user to double check

Thanks for reporting