Bug 192528 - pwd_mkdb fails if /etc/shells contains duplicates
Summary: pwd_mkdb fails if /etc/shells contains duplicates
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 10.0-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-09 16:14 UTC by David
Modified: 2018-02-27 19:28 UTC (History)
3 users (show)

See Also:
kevans: mfc-stable11+
kevans: mfc-stable10-
emaste: mfc-stable9-


Attachments
/etc/shells with duplicate entries (7.17 KB, text/plain)
2014-08-09 16:14 UTC, David
no flags Details
getusershell.patch against _local_initshells overflow (431 bytes, patch)
2015-05-14 20:12 UTC, tmwalaszek
no flags Details | Diff
Stripped down example of one failure case (1.02 KB, text/plain)
2017-05-12 03:30 UTC, Kyle Evans
no flags Details
Stripped down example of second failure case (1.02 KB, text/plain)
2017-05-12 03:30 UTC, Kyle Evans
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David 2014-08-09 16:14:56 UTC
Created attachment 145573 [details]
/etc/shells with duplicate entries

My FreeBSD 10 system stopped being able to add new users. The 'adduser' command would fail with "User 'username' disappeared during update". This was causing me much grief because service users required by packages would not be created which would prevent the associated service from starting.

The cause was that the command "pwd_mkdb -p /etc/master.passwd" was aborting. The abort is apparently caused by duplicate entries in /etc/shells. My /etc/shells file contained many duplicates (attached).

I removed all duplicates, and then the "pwd_mkdb" command was able to complete without aborting.

I guess there are two problems here:

1) /etc/shells is populated with duplicates. I don't know what process did this.
2) pwd_mkdb fails if there are duplicates.
Comment 2 tmwalaszek 2015-05-14 20:12:31 UTC
Created attachment 156784 [details]
getusershell.patch against _local_initshells overflow
Comment 3 tmwalaszek 2015-05-14 20:14:35 UTC
Hi,
Problem is in function _local_initshells in file lib/libc/gen/getusershell.c

Function is reading /etc/shells and put every shell path to line[MAXPATHLEN + 1]
It looks like this '/bin/csh\0/usr/local/bin/zsh\0/usr/local/bin/bash', it does
not clean this array (and not set pointer at the beggining) on every iteration so if we have many lines in /etc/shells it will overflow.

Simple patch is in the attachment. Works for me ;)
Comment 4 tmwalaszek 2015-05-22 23:12:44 UTC
This bug also affects chpass, getent shells and /usr/libexec/ftpd. Generally everything that uses setusershell (which executes _local_initshells) with large /etc/shells is affected.
Comment 5 Kyle Evans freebsd_committer freebsd_triage 2017-05-12 03:30:27 UTC
Created attachment 182527 [details]
Stripped down example of one failure case
Comment 6 Kyle Evans freebsd_committer freebsd_triage 2017-05-12 03:30:50 UTC
Created attachment 182528 [details]
Stripped down example of second failure case
Comment 7 Kyle Evans freebsd_committer freebsd_triage 2017-05-12 03:32:27 UTC
Hi,

I've created the following review for this: https://reviews.freebsd.org/D10690

memset()'ing the line isn't necessary every iteration since fgets() should just overwrite as necessary and null-terminate properly. I've also hit the second case where a line ends in a comment or with no valid shell.

Cheers,

Kyle Evans
Comment 8 commit-hook freebsd_committer freebsd_triage 2017-05-15 17:57:51 UTC
A commit references this bug:

Author: emaste
Date: Mon May 15 17:57:09 UTC 2017
New revision: 318304
URL: https://svnweb.freebsd.org/changeset/base/318304

Log:
  getusershell: don't write past end of line buffer reading local shells

  _local_initshells did not reset cp to the beginning of the line buffer
  for every iteration that it called fgets(3), leading to writing past the
  end of line with fairly long /etc/shells or excessively long line
  lengths. Correct this by properly resetting cp.

  PR:		192528
  Submitted by:	Kyle Evans <kevans91@ksu.edu>
  Reviewed by:	cem, jilles
  Differential Revision:	https://reviews.freebsd.org/D10690

Changes:
  head/lib/libc/gen/getusershell.c
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-02-27 19:24:55 UTC
A commit references this bug:

Author: kevans
Date: Tue Feb 27 19:24:06 UTC 2018
New revision: 330081
URL: https://svnweb.freebsd.org/changeset/base/330081

Log:
  MFC r318304: getusershell: don't write paste end of buffer reading shells

  _local_initshells did not reset cp to the beginning of the line buffer for
  every iteration that it called fgets(3), leading to writing past the end of
  line with fairly long /etc/shells or excessively long line lengths. Correct
  this by properly resetting cp.

  PR:		192528

Changes:
_U  stable/11/
  stable/11/lib/libc/gen/getusershell.c
Comment 10 Kyle Evans freebsd_committer freebsd_triage 2018-02-27 19:28:07 UTC
I don't currently intend to MFC this to stable/10, but I will upon request.