shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

Bogus suauth(5) parser or documentation in src/suauth.c #1039

Closed alejandro-colomar closed 1 week ago

alejandro-colomar commented 1 week ago

https://github.com/shadow-maint/shadow/blob/53ea42e67f6ed3ca1f7eae69a3992774be627a45/src/suauth.c#L143-L149

suauth(5) specifies that the separator is a comma ',', and it insists several times that white space is not allowed.

See https://www.man7.org/linux/man-pages/man5/suauth.5.html

However, this parser seems to be treating white space as a valid separator, just as if a comma had been written. This behavior goes back to commit 1:

https://github.com/shadow-maint/shadow/commit/45c6603cc86c5881b00ac40e0f9fe548c30ff6be

The author probably meant to consider the delimiter to be the string ", ", but that's not how strtok(3) works: strtok(3) considers any of the characters in the 2nd argument to be a valid delimiter. That is, it is implemented internally with strspn(3), not with strstr(3).

alejandro-colomar commented 1 week ago

Here's an example file exctracted from the manual page:

                 # sample /etc/suauth file
                 #
                 # A couple of privileged usernames may
                 # su to root with their own password.
                 #
                 root:chris,birddog:OWNPASS
                 #
                 # Anyone else may not su to root unless in
                 # group wheel. This is how BSD does things.
                 #
                 root:ALL EXCEPT GROUP wheel:DENY
                 #
                 # Perhaps terry and birddog are accounts
                 # owned by the same person.
                 # Access can be arranged between them
                 # with no password.
                 #
                 terry:birddog:NOPASS
                 birddog:terry:NOPASS
                 #

The line

                 root:ALL EXCEPT GROUP wheel:DENY

contains whitespace. How should that be treated? Do we want strtok(3) to split right there, or should it be a single token?

The source code seems to split those, so then, do we treat space as a valid delimiter? Should the man page be changed? It seems inconsistent.

alejandro-colomar commented 1 week ago

Hmmm, I think I misunderstood.