summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlejandro Colomar <alx@kernel.org>2024-02-05 13:14:13 +0100
committerAlejandro Colomar <alx@kernel.org>2024-02-14 04:10:03 +0100
commitcc2970c3a1639a59abaf92f56cc4288d9ebc8d57 (patch)
treed278ff93476120b9f063eec481f8240ee51170cd
parentdbdda2a48a77d03e3566a1b1694fb597d63b81b0 (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.c19
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 */