From: Alejandro Colomar <alx@kernel.org>
Date: Mon, 5 Feb 2024 12:14:13 +0000 (+0100)
Subject: src/login.c: Fix off-by-one buggs
X-Git-Tag: 4.15.0-rc2~6
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6551709e96b2bc6f084fdf170ad5bcc11f0038ab;p=thirdparty%2Fshadow.git

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

diff --git a/src/login.c b/src/login.c
index 77c58d4ee..c1bc5633d 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);
@@ -879,15 +881,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 */