Source: pam
Version: 1.1.8-3.6
Severity: normal
Hi maintainers,
When looking for some strange behaviour pam_unix, I came across a possible typo
in the securetty patch [1]. I'll highlight the relevant parts of the code
below.
if (isdigit(uttyname[0])) {
snprintf(ptname, sizeof(ptname), "pts/%s", uttyname);
} else {
ptname[0] = '\0';
}
This means that either:
1) uttyname starts with a digit and ptname is "pts/$uttyname".
2) utty name does not start with a digit and ptsname is "".
Then there is:
retval = 1;
while ((fgets(ttyfileline,sizeof(ttyfileline)-1, ttyfile) != NULL)
&& retval) {
if(ttyfileline[strlen(ttyfileline) - 1] == '\n')
ttyfileline[strlen(ttyfileline) - 1] = '\0';
retval = ( strcmp(ttyfileline,uttyname)
&& (!ptname[0] || strcmp(ptname, uttyname)) );
}
And in particular, this part of the check:
(!ptname[0] || strcmp(ptname, uttyname))
In case 1) above, ptname is not empty and ptname can never equal uttyname, so
this resolves to (false || non-zero) == true. In case 2) above, this resolves
to (true || whatever) == true. Hence, I believe this part of the check just
means && true and could be omitted.
But more likely, there's a typo in this line and it should have read:
(!ptname[0] || strcmp(ptname, ttyfileline))
This would check each line from the securetty file against both uttyname and
ptname, instead of just against uttyname.
I'm not sure if the ptname part is actually needed at all, since this code
seems to work just fine without it for /dev/pts devices (but perhaps not in all
cases, I'm not sure what forms uttyname can have).
[1]:
https://alioth.debian.org/scm/loggerhead/pkg-pam/debian/sid/view/head:/debian/patches-applied/054_pam_security_abstract_securetty_handling#L162
Gr.
Matthijs
