From: Samanta Navarro <ferivoz@riseup.net>
Date: Tue, 23 May 2023 11:55:09 +0000 (+0000)
Subject: su: Prevent stack overflow in check_perms
X-Git-Tag: 4.14.0-rc1~73
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a116e20c76f36d6f72f101508f634b6c1bddb9be;p=thirdparty%2Fshadow.git

su: Prevent stack overflow in check_perms

This is no real world security fix.

The overflow could occur if too many layered subsystems are encountered
because the function check_perms calls itself recursively.

It would already take a misconfigured system for this to achieve it.

Use an iterative approach by calling the do_check_perms in a loop
instead of calling itself recursively.

As a side note: At least GCC 13 optimized this code and already uses
a jmp in its assembler code. I could only see the stack overflow by
activating address sanitizer which prevented the optimization.

Co-developed-by: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Samanta Navarro <ferivoz@riseup.net>
---

diff --git a/src/su.c b/src/su.c
index 3402f9bea..3a8153e25 100644
--- a/src/su.c
+++ b/src/su.c
@@ -115,6 +115,7 @@ static bool iswheel (const char *);
 static bool restricted_shell (const char *shellname);
 NORETURN static void su_failure (const char *tty, bool su_to_root);
 static /*@only@*/struct passwd * check_perms (void);
+static /*@only@*/struct passwd * do_check_perms (void);
 #ifdef USE_PAM
 static void check_perms_pam (const struct passwd *pw);
 #else				/* !USE_PAM */
@@ -618,10 +619,25 @@ static void check_perms_nopam (const struct passwd *pw)
 /*
  * check_perms - check permissions to switch to the user 'name'
  *
- *	In case of subsystem login, the user is first authenticated in the
- *	caller's root subsystem, and then in the user's target subsystem.
+ *	The user is authenticated in all subsystems iterately.
  */
 static /*@only@*/struct passwd * check_perms (void)
+{
+	struct passwd *pw = NULL;
+
+	while (pw == NULL)
+		pw = do_check_perms();
+	return pw;
+}
+
+/*
+ * do_check_perms - check permissions to switch to the user 'name'
+ *
+ *	The user is authenticated in current subsystem, if any. Returns
+ *	NULL if permissions have to be checked in next subsystem, in
+ *	which case the subsystem has already been entered.
+ */
+static /*@only@*/struct passwd * do_check_perms (void)
 {
 #ifdef USE_PAM
 	const void *tmp_name;
@@ -692,7 +708,7 @@ static /*@only@*/struct passwd * check_perms (void)
 		endpwent ();		/* close the old password databases */
 		endspent ();
 		pw_free (pw);
-		return check_perms ();	/* authenticate in the subsystem */
+		return NULL;	/* authenticate in the subsystem */
 	}
 
 	return pw;