Bug 221416

Summary: pw useradd accepts invalid user names
Product: Base System Reporter: Fabian Keil <fk>
Component: binAssignee: Ed Maste <emaste>
Status: Closed FIXED    
Severity: Affects Some People CC: emaste
Priority: --- Keywords: patch, regression
Version: CURRENTFlags: fk: mfc-stable11?
fk: mfc-stable10?
Hardware: Any   
OS: Any   
Attachments:
Description Flags
pw useradd: Validate the user name before creating the entry none

Description Fabian Keil 2017-08-11 11:42:44 UTC
Created attachment 185282 [details]
pw useradd: Validate the user name before creating the entry

When the -g option is used, pw useradd doesn't validate the user name.

A command like:
    pw useradd -u 1234 -g 1234 -n 'test user'
creates an invalid user.

The "-g 1234" is relevant, without it the name is rejected as expected:
    
    [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
    pw: invalid character ` ' at position 4 in userid/group name
    
Bug unintentionally found with a salt config without explicit name entry:
    
    test user:
      user.present:
        - uid: 1234
        - gid: 1234
        - fullname: Test user
        - shell: /usr/local/bin/bash
        - home: /home/test
        - groups:
          - wheel
          - salt
    
"Luckily" salt modules rarely bother with input validation either ...

This regression was introduced when pw was refactored in 2015.

The attached patch fixes the issue and adds regression tests.
    
Obtained from: ElectroBSD
Comment 1 commit-hook freebsd_committer 2017-08-19 00:33:20 UTC
A commit references this bug:

Author: emaste
Date: Sat Aug 19 00:32:26 UTC 2017
New revision: 322678
URL: https://svnweb.freebsd.org/changeset/base/322678

Log:
  pw useradd: Validate the user name before creating the entry

  Previouly it was possible to create users with spaces in the name with:
  pw useradd -u 1234 -g 1234 -n 'test user'

  The "-g 1234" is relevant, without it the name was already rejected
  as expected:

  [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
  pw: invalid character ` ' at position 4 in userid/group name

  Bug unintentionally found with a salt config without explicit name entry:

  test user:
    user.present:
      - uid: 1234
      - gid: 1234
      - fullname: Test user
      - shell: /usr/local/bin/bash
      - home: /home/test
      - groups:
        - wheel
        - salt

  "Luckily" salt modules rarely bother with input validation either ...

  PR:		221416
  Submitted by:	Fabian Keil
  Obtained from:	ElectroBSD
  MFC after:	1 week

Changes:
  head/usr.sbin/pw/pw_user.c
  head/usr.sbin/pw/tests/pw_useradd_test.sh
Comment 2 commit-hook freebsd_committer 2017-09-08 21:02:52 UTC
A commit references this bug:

Author: emaste
Date: Fri Sep  8 21:02:16 UTC 2017
New revision: 323332
URL: https://svnweb.freebsd.org/changeset/base/323332

Log:
  MFC r322678: pw useradd: Validate the user name before creating the entry

  Previouly it was possible to create users with spaces in the name with:
  pw useradd -u 1234 -g 1234 -n 'test user'

  The "-g 1234" is relevant, without it the name was already rejected
  as expected:

  [fk@test ~]$ sudo pw useradd -u 1234 -n 'test user'
  pw: invalid character ` ' at position 4 in userid/group name

  Bug unintentionally found with a salt config without explicit name entry:

  test user:
    user.present:
      - uid: 1234
      - gid: 1234
      - fullname: Test user
      - shell: /usr/local/bin/bash
      - home: /home/test
      - groups:
        - wheel
        - salt

  "Luckily" salt modules rarely bother with input validation either ...

  PR:		221416
  Submitted by:	Fabian Keil
  Approved by:	re (kib)
  Obtained from:	ElectroBSD

Changes:
_U  stable/10/
  stable/10/usr.sbin/pw/pw_user.c
  stable/10/usr.sbin/pw/tests/pw_useradd_test.sh
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:03 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.