Skip to content

Conversation

@alejandro-colomar
Copy link
Collaborator

@alejandro-colomar alejandro-colomar commented Jan 9, 2026

Regression fixes:

Security bug fixes:

Other changes:

  • vipw(8):
    • Set SIGCHLD before fork(2). This prevents a possible race
      condition, although this is only for making the code more robust;
      we believe that the code was correct.
      See vipw: set SIGCHLD before fork #1484.

Closes: #1487

@alejandro-colomar
Copy link
Collaborator Author

Cc: @stoeckmann

@alejandro-colomar alejandro-colomar linked an issue Jan 9, 2026 that may be closed by this pull request
@alejandro-colomar alejandro-colomar force-pushed the rel4.19.1 branch 2 times, most recently from ee7101b to 54fc9a9 Compare January 9, 2026 13:49
Regression fixes:

-  chpasswd(8):
   -  Don't reject leading '!' in password hashes or a hash consisting
      of "*".  These were accidentally rejected in 4.19.0.
      See <shadow-maint#1483>
      and <shadow-maint#1486>.

Security bug fixes:

-  vipw(8):
   -  Avoid predictable names for temporary files.
      This allowed anyone to read the contents of /etc/shadow.
      See <shadow-maint#1485>.

Other changes:

-  vipw(8):
   -  Set SIGCHLD before fork(2).  This prevents a possible race
      condition, although this is only for making the code more robust;
      we believe that the code was correct.
      See <shadow-maint#1484>.

Closes: <shadow-maint#1487>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
@stoeckmann
Copy link
Contributor

stoeckmann commented Jan 9, 2026

  * Avoid predictable names for temporary files.
    This allowed anyone to read the contents of /etc/shadow.
    See [vipw: Use fmkomstemp for temporary file #1485](https://github.com/shadow-maint/shadow/pull/1485).

The description is far too scary for what's really going on. If you would want to keep that sentence, it would be
"This allowed anyone with write access to /etc to expose the contents of /etc/shadow to other users."

Since this is normally just root, it would be
"An accidental file creation by root could allow exposure of the contents of /etc/shadow to other users."

So, if you want to have a description, it's much better to link to CWE-377 or maybe CAPEC-149 to show that this is a defensive measure to break a chain of required attacks to get the contents of /etc/shadow.

  * Set SIGCHLD before fork(2).  This prevents a possible race
    condition, although this is only for making the code more robust;
    we believe that the code was correct.
    See [vipw: set SIGCHLD before fork #1484](https://github.com/shadow-maint/shadow/pull/1484).

It was not correct. But it's a race condition with a very small time window. It took a sleep() for me to follow the proof of concept of the previous pull request to show that the SIGCHLD adjustment was at a wrong location.

@alejandro-colomar
Copy link
Collaborator Author

  * Avoid predictable names for temporary files.
    This allowed anyone to read the contents of /etc/shadow.
    See [vipw: Use fmkomstemp for temporary file #1485](https://github.com/shadow-maint/shadow/pull/1485).

The description is far too scary for what's really going on. If you would want to keep that sentence, it would be "This allowed anyone with write access to /etc to expose the contents of /etc/shadow to other users."

Since this is normally just root, it would be "An accidental file creation by root could allow exposure of the contents of /etc/shadow to other users."

So, if you want to have a description, it's much better to link to CWE-377 or maybe CAPEC-149 to show that this is a defensive measure to break a chain of required attacks to get the contents of /etc/shadow.

Thanks! I'll add those.

  * Set SIGCHLD before fork(2).  This prevents a possible race
    condition, although this is only for making the code more robust;
    we believe that the code was correct.
    See [vipw: set SIGCHLD before fork #1484](https://github.com/shadow-maint/shadow/pull/1484).

It was not correct. But it's a race condition with a very small time window. It took a sleep() for me to follow the proof of concept of the previous pull request to show that the SIGCHLD adjustment was at a wrong location.

Hmmm, thanks! I'll fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4.19.1 Release plan - 2026-01

2 participants