43008: Improve code to remove privileges.

Remove warnings of unused values as we always check the finally
result later.

Put segid before setuid as the setgid could fail if UID
no longer privileged.

1 files changed, 24 insertions, 7 deletions

diff --git a/Src/options.c b/Src/options.c
index 590652ea9..600b649e4 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -769,15 +769,32 @@ dosetopt(int optno, int value, int force, char *new_opts)
     } else if(optno == PRIVILEGED && !value) {
 	/* unsetting PRIVILEGED causes the shell to make itself unprivileged */
 #ifdef HAVE_SETUID
-	setuid(getuid());
-	setgid(getgid());
-        if (setuid(getuid())) {
-            zwarn("failed to change user ID: %e", errno);
-            return -1;
-	} else if (setgid(getgid())) {
+	int ignore_err;
+	errno = 0;
+	/*
+	 * Set the GID first as if we set the UID to non-privileged it
+	 * might be impossible to restore the GID.
+	 *
+	 * Some OSes (possibly no longer around) have been known to
+	 * fail silently the first time, so we attempt the change twice.
+	 * If it fails we are guaranteed to pick this up the second
+	 * time, so ignore the first time.
+	 *
+	 * Some versions of gcc make it hard to ignore the results the
+	 * first time, hence the following.  (These are probably not
+	 * systems that require the doubled calls.)
+	 */
+	ignore_err = setgid(getgid());
+	(void)ignore_err;
+	ignore_err = setuid(getuid());
+	(void)ignore_err;
+	if (setgid(getgid())) {
             zwarn("failed to change group ID: %e", errno);
             return -1;
-        }
+        } else if (setuid(getuid())) {
+            zwarn("failed to change user ID: %e", errno);
+            return -1;
+	}
 #else
         zwarn("setuid not available");
         return -1;