shadow-maint/shadow

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

alejandro-colomar opened this issue · 2 comments

shadow/src/suauth.c

Lines 143 to 149 in 53ea42e

const char split[] = ", ";
char *tok;
int state = 0;
for (tok = strtok (list, split); tok != NULL;
tok = strtok (NULL, split)) {

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:

45c6603

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).

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.

Hmmm, I think I misunderstood.