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 3402f9bea1485607f47ac69e78b36b5beb15e95f..3a8153e25f1c9f16c2cc95f5bc36bb6b4ba6263d 100644 (file)
--- 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;