Bug 245342 - adduser.sh silently sets a password different from the input if it contains leading/trailing spaces
Summary: adduser.sh silently sets a password different from the input if it contains l...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Kyle Evans
URL:
Keywords: needs-patch, needs-qa
Depends on:
Blocks:
 
Reported: 2020-04-04 03:13 UTC by Mario Baldini
Modified: 2020-04-27 19:31 UTC (History)
3 users (show)

See Also:
kevans: mfc-stable12+
kevans: mfc-stable11+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Baldini 2020-04-04 03:13:39 UTC
Hello all, 

TL;DR; 

`adduser.sh` script silently leads to unexpected system configuration if the password contains leading or trailing space. It ends successfully, but configuring the account with a password different from what was input by the user.
The behavior is not clearly informed to the user (during the process itself; nor in the Handbook). 


Steps to reproduce: (tested on FreeBSD 13.0)
- Create a user account with `useradd` (ie. during the installation process)
- In the user password insertion, provide one with leading/trailing space
- Attempt to login afterward, with the previously inserted password
- Error: login not possible due wrong password
(can be confirmed by providing the same string but without the leading/trailing password, it will login as expected)


Expected behavior: 
- Password set process should support leading/trailing spaces;
- Or explicitly warn the user and request a different password (but not proceed silently defining a password that was not the input (and confirmed) string)



Accordinly to line 736 of freebsd/usr.sbin/adduser/adduser.sh 

    echo -n "Enter password: "
    read -r upass
    echo''
    echo -n "Enter password again: "
    read -r _passconfirm


and `bash` documentation `read` section:

     The trailing newline is deleted from the line and the line is split as described in the section on White Space Splitting (Field Splitting) above, and the pieces are assigned to the variables in order.
     (...)
     White Space Splitting: 
      Whitespace in IFS at the beginning or end of a word is discarded.


Apparently, removing any leading/trailing space is the expected behavior of `read -r upass` command itself, but should not be the expected one of the `adduser.sh` process as a whole. Also, the FreeBSD Handbook `3.3.2.1. adduser` or `Example 3.2, “Adding a User on FreeBSD”` do not mention this. In a new install the user may simply complete it without any error and later be locked out of the machine in this scenario. (I eventually just found guessing the issue and trying the pwd without the spaces).


I guess the process workings itself should not be changed (since it conflicts with the regular `read` "word split" behavior), also due being an edge case. But in any case, the user should be adequately informed and the process continued only if the string set as the pwd to be exacly what the user inserted (and confirmed).


PS: I search the bugzilla archive and other forums but did not find any previous discussion that addresses this particular case. Please let me know if there is a better channel or I am misunderstanding something about adduser script. 


Thank you all,

Best regards,

Mario Baldini




Error logs:
/bin/sh

$ sudo adduser
Username: somename
Full name: Some
Uid (Leave empty for default): 
Login group [somename]: 
Login group is somename. Invite somename into other groups? []: 
Login class [default]: 
Shell (sh csh tcsh zsh rzsh git-shell bash rbash nologin) [sh]: 
Home directory [/home/somename]: 
Home directory permissions (Leave empty for default): 
Use password-based authentication? [yes]: 
Use an empty password? (yes/no) [no]: 
Use a random password? (yes/no) [no]: 
Enter password: 
Enter password again: 
Lock out the account after creation? [no]: 
Username   : somename
Password   : *****
Full Name  : Some
Uid        : 1002
Class      : 
Groups     : somename 
Home       : /home/somename
Home Mode  : 
Shell      : /bin/sh
Locked     : no
OK? (yes/no): yes
adduser: INFO: Successfully added (somename) to the user database.
Add another user? (yes/no): no
Goodbye!
$ 
# - user account was created successfully
# - the password set for is is different from what was input (if the input contained leading/trailing space)
# - user is not aware of this mismatch
Comment 1 commit-hook freebsd_committer 2020-04-05 19:26:46 UTC
A commit references this bug:

Author: kevans
Date: Sun Apr  5 19:25:46 UTC 2020
New revision: 359642
URL: https://svnweb.freebsd.org/changeset/base/359642

Log:
  adduser: allow standard IFS characters in passwords

  Notably, the default IFS contains space/tab, thus any leading/trailing
  whitespace characters tend to be removed.

  Set IFS= for just the read lines to mitigate this, allowing the user to be
  less surprised when their leading/trailing spaces weren't actually captured
  in the password as they are with other means of setting a user's password.

  PR:		245342
  Submitted by:	dereks_lifeofadishwasher.com
  Reviewed by:	jilles
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D24292

Changes:
  head/usr.sbin/adduser/adduser.sh
Comment 2 commit-hook freebsd_committer 2020-04-27 19:30:19 UTC
A commit references this bug:

Author: kevans
Date: Mon Apr 27 19:29:48 UTC 2020
New revision: 360390
URL: https://svnweb.freebsd.org/changeset/base/360390

Log:
  MFC r359642: adduser: allow standard IFS characters in passwords

  Notably, the default IFS contains space/tab, thus any leading/trailing
  whitespace characters tend to be removed.

  Set IFS= for just the read lines to mitigate this, allowing the user to be
  less surprised when their leading/trailing spaces weren't actually captured
  in the password as they are with other means of setting a user's password.

  PR:		245342

Changes:
_U  stable/11/
  stable/11/usr.sbin/adduser/adduser.sh
_U  stable/12/
  stable/12/usr.sbin/adduser/adduser.sh