diff options
author | Alejandro Colomar <alx@kernel.org> | 2024-02-05 13:14:13 +0100 |
---|---|---|
committer | Alejandro Colomar <alx@kernel.org> | 2024-02-14 04:10:03 +0100 |
commit | cc2970c3a1639a59abaf92f56cc4288d9ebc8d57 (patch) | |
tree | d278ff93476120b9f063eec481f8240ee51170cd | |
parent | dbdda2a48a77d03e3566a1b1694fb597d63b81b0 (diff) |
src/login.c: Fix off-by-one buggs
Before 3b7cc053872c ("lib: replace `USER_NAME_MAX_LENGTH` macro"), this
code did use a length. It used a utmp(5) fixed-width buffer, so the
length matches the buffer size (there was no terminating NUL byte).
However, sysconf(_SC_LOGIN_NAME_MAX) returns a buffer size that accounts
for the terminating null byte; see sysconf(3). Thus, the commit that
introduced the call to sysconf(3), should have taken that detail into
account.
403a2e3771be ("lib/chkname.c: Take NUL byte into account"), by Tobias,
caught that bug in <lib/chkname.c>, but missed that the same commit that
introduced that bug, introduced the same bug in two other places.
This fixes all remaining calls to sysconf(_SC_LOGIN_NAME_MAX).
I still observe some suspicious code after this fix:
if (do_rlogin(hostname, username, max_size - 1, term, sizeof(term)))
...
login_prompt(username, max_size - 1);
We're passing size-1 to functions that want a size. But since the fix
to those will be different, let's do that in the following commits.
Link: <https://github.com/shadow-maint/shadow/pull/935>
Link: <https://github.com/shadow-maint/shadow/issues/920#issuecomment-1926002209>
Link: <https://github.com/shadow-maint/shadow/pull/757>
Link: <https://github.com/shadow-maint/shadow/issues/674>
See-also: 403a2e3771be ("lib/chkname.c: Take NUL byte into account")
Fixes: 3b7cc053872c ("lib: replace `USER_NAME_MAX_LENGTH` macro")
Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com>
Cc: Tobias Stoeckmann <tobias@stoeckmann.org>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
Cherry-picked-from: 6551709e96b2 ("src/login.c: Fix off-by-one buggs")
Link: <https://github.com/shadow-maint/shadow/pull/936>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
-rw-r--r-- | src/login.c | 19 |
1 files changed, 11 insertions, 8 deletions
diff --git a/src/login.c b/src/login.c index b5562923..9ab2678f 100644 --- a/src/login.c +++ b/src/login.c @@ -572,11 +572,13 @@ int main (int argc, char **argv) } #ifdef RLOGIN if (rflg) { - size_t max_size = sysconf(_SC_LOGIN_NAME_MAX); + size_t max_size = sysconf(_SC_LOGIN_NAME_MAX); + assert (NULL == username); - username = XMALLOC(max_size + 1, char); - username[max_size] = '\0'; - if (do_rlogin (hostname, username, max_size, term, sizeof term)) { + username = XMALLOC(max_size, char); + username[max_size - 1] = '\0'; + if (do_rlogin(hostname, username, max_size - 1, term, sizeof(term))) + { preauth_flag = true; } else { free (username); @@ -885,15 +887,16 @@ int main (int argc, char **argv) failed = false; /* haven't failed authentication yet */ if (NULL == username) { /* need to get a login id */ - size_t max_size = sysconf(_SC_LOGIN_NAME_MAX); + size_t max_size = sysconf(_SC_LOGIN_NAME_MAX); + if (subroot) { closelog (); exit (1); } preauth_flag = false; - username = XMALLOC(max_size + 1, char); - username[max_size] = '\0'; - login_prompt (username, max_size); + username = XMALLOC(max_size, char); + username[max_size - 1] = '\0'; + login_prompt(username, max_size - 1); if ('\0' == username[0]) { /* Prompt for a new login */ |