Bug 221416 - pw useradd accepts invalid user names
Summary: pw useradd accepts invalid user names
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Ed Maste
URL:
Keywords: patch, regression
Depends on:
Blocks:
 
Reported: 2017-08-11 11:42 UTC by Fabian Keil
Modified: 2018-08-29 20:00 UTC (History)
1 user (show)

See Also:
fk: mfc-stable11?
fk: mfc-stable10?


Attachments
pw useradd: Validate the user name before creating the entry (5.67 KB, patch)
2017-08-11 11:42 UTC, Fabian Keil
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 freebsd_triage 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 freebsd_triage 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.