Improve PRIVILEGED fixes

- Fix retval handling in bin_setopt()

- Don't skip_setuid / skip_setgid.  It's not our place to optimize away noops
  (that might not even _be_ noops; they might change the saved uid…).

- Remove HAVE_* guard checks around functions that are used unguarded elsewhere.

- Use bsd-setres_id.c from OpenSSH to provide setresuid() / setresgid()
  everywhere, and thus simplify the ifdef soup.  Fix some preëxisting
  bugs in the macro definitions of setuid() (do we still need that one?).

- Fix zwarning() format codes for variadic arguments type safety

- Restored a comment from HEAD

- Fix failure modes around initgroups()

- Compared privilege restoration code with OpenSSH's permanently_drop_uid() and
  updated as needed

- Add E01 PRIVILEGED sanity checks

6 files changed, 292 insertions, 102 deletions

diff --git a/Src/openssh_bsd_setres_id.c b/Src/openssh_bsd_setres_id.c
new file mode 100644
index 000000000..65e91a40c
--- /dev/null
+++ b/Src/openssh_bsd_setres_id.c
@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2012 Darren Tucker (dtucker at zip com au).
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+/*
+ * openssh_bsd_setres_id.c - setresuid() and setresgid() wrappers
+ *
+ * This file is part of zsh, the Z shell.
+ *
+ * It is based on the file openbsd-compat/bsd-setres_id.c in OpenSSH 7.9p1,
+ * which is subject to the copyright notice above.  The zsh modifications are
+ * licensed as follows:
+ *
+ * Copyright (c) 2019 Daniel Shahaf
+ * All rights reserved.
+ *
+ * Permission is hereby granted, without written agreement and without
+ * license or royalty fees, to use, copy, modify, and distribute this
+ * software and to distribute modified versions of this software for any
+ * purpose, provided that the above copyright notice and the following
+ * two paragraphs appear in all copies of this software.
+ *
+ * In no event shall Daniel Shahaf or the Zsh Development Group be liable
+ * to any party for direct, indirect, special, incidental, or consequential
+ * damages arising out of the use of this software and its documentation,
+ * even if Daniel Shahaf and the Zsh Development Group have been advised of
+ * the possibility of such damage.
+ *
+ * Daniel Shahaf and the Zsh Development Group specifically disclaim any
+ * warranties, including, but not limited to, the implied warranties of
+ * merchantability and fitness for a particular purpose.  The software
+ * provided hereunder is on an "as is" basis, and Daniel Shahaf and the
+ * Zsh Development Group have no obligation to provide maintenance,
+ * support, updates, enhancements, or modifications.
+ *
+ */
+
+
+#include <sys/types.h>
+
+#include <stdarg.h>
+#include <unistd.h>
+#include <string.h>
+
+#include "zsh.mdh"
+
+#if defined(ZSH_IMPLEMENT_SETRESGID) || defined(BROKEN_SETRESGID)
+int
+setresgid(gid_t rgid, gid_t egid, gid_t sgid)
+{
+	int ret = 0, saved_errno;
+
+	if (rgid != sgid) {
+		errno = ENOSYS;
+		return -1;
+	}
+#if defined(ZSH_HAVE_NATIVE_SETREGID) && !defined(BROKEN_SETREGID)
+	if (setregid(rgid, egid) < 0) {
+		saved_errno = errno;
+		zwarnnam("setregid", "to gid %L: %e", (long)rgid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+#else
+	if (setegid(egid) < 0) {
+		saved_errno = errno;
+		zwarnnam("setegid", "to gid %L: %e", (long)(unsigned int)egid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+	if (setgid(rgid) < 0) {
+		saved_errno = errno;
+		zwarnnam("setgid", "to gid %L: %e", (long)rgid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+#endif
+	return ret;
+}
+#endif
+
+#if defined(ZSH_IMPLEMENT_SETRESUID) || defined(BROKEN_SETRESUID)
+int
+setresuid(uid_t ruid, uid_t euid, uid_t suid)
+{
+	int ret = 0, saved_errno;
+
+	if (ruid != suid) {
+		errno = ENOSYS;
+		return -1;
+	}
+#if defined(ZSH_HAVE_NATIVE_SETREUID) && !defined(BROKEN_SETREUID)
+	if (setreuid(ruid, euid) < 0) {
+		saved_errno = errno;
+		zwarnnam("setreuid", "to uid %L: %e", (long)ruid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+#else
+
+# ifndef SETEUID_BREAKS_SETUID
+	if (seteuid(euid) < 0) {
+		saved_errno = errno;
+		zwarnnam("seteuid", "to uid %L: %e", (long)euid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+# endif
+	if (setuid(ruid) < 0) {
+		saved_errno = errno;
+		zwarnnam("setuid", "to uid %L: %e", (long)ruid, errno);
+		errno = saved_errno;
+		ret = -1;
+	}
+#endif
+	return ret;
+}
+#endif
diff --git a/Src/options.c b/Src/options.c
index 328cf318b..425e27dc2 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -607,25 +607,21 @@ bin_setopt(char *nam, char **args, UNUSED(Options ops), int isun)
 		}
 		if(!(optno = optlookup(*args))) {
 		    zwarnnam(nam, "no such option: %s", *args);
-		    retval = 1;
-		} else {
-		    retval = !!dosetopt(optno, action, 0, opts);
-		    if (retval) {
-			zwarnnam(nam, "can't change option: %s", *args);
-		    }
+		    retval |= 1;
+		} else if (dosetopt(optno, action, 0, opts)) {
+		    zwarnnam(nam, "can't change option: %s", *args);
+		    retval |= 1;
 		}
 		break;
 	    } else if(**args == 'm') {
 		match = 1;
 	    } else {
-	    	if (!(optno = optlookupc(**args))) {
+		if (!(optno = optlookupc(**args))) {
 		    zwarnnam(nam, "bad option: -%c", **args);
-		    retval = 1;
-		} else {
-		    retval = !!dosetopt(optno, action, 0, opts);
-		    if (retval) {
-			zwarnnam(nam, "can't change option: -%c", **args);
-		    }
+		    retval |= 1;
+		} else if (dosetopt(optno, action, 0, opts)) {
+		    zwarnnam(nam, "can't change option: -%c", **args);
+		    retval |= 1;
 		}
 	    }
 	}
@@ -638,12 +634,10 @@ bin_setopt(char *nam, char **args, UNUSED(Options ops), int isun)
 	while (*args) {
 	    if(!(optno = optlookup(*args++))) {
 		zwarnnam(nam, "no such option: %s", args[-1]);
-		retval = 1;
-	    } else {
-		retval = !!dosetopt(optno, !isun, 0, opts);
-		if (retval) {
-		    zwarnnam(nam, "can't change option: %s", args[-1]);
-		}
+		retval |= 1;
+	    } else if (dosetopt(optno, !isun, 0, opts)) {
+		zwarnnam(nam, "can't change option: %s", args[-1]);
+		retval |= 1;
 	    }
 	}
     } else {
@@ -667,7 +661,7 @@ bin_setopt(char *nam, char **args, UNUSED(Options ops), int isun)
 	    tokenize(s);
 	    if (!(pprog = patcompile(s, PAT_HEAPDUP, NULL))) {
 		zwarnnam(nam, "bad pattern: %s", *args);
-		retval = 1;
+		retval |= 1;
 		break;
 	    }
 	    /* Loop over expansions. */
@@ -787,100 +781,92 @@ dosetopt(int optno, int value, int force, char *new_opts)
     } else if(optno == PRIVILEGED && !value) {
 	/* unsetting PRIVILEGED causes the shell to make itself unprivileged */
 
-	int skip_setuid = 0;
-	int skip_setgid = 0;
-
-#if defined(HAVE_GETEGID) && defined(HAVE_SETGID) && defined(HAVE_GETUID)
-	int orig_egid = getegid();
-#endif
+	/* If set, return -1 so lastval will be non-zero. */
+	int failed = 0;
 
-#if defined(HAVE_GETEUID) && defined(HAVE_GETUID)
-	if (geteuid() == getuid()) {
-	    skip_setuid = 1;
-	}
+#ifdef HAVE_SETUID
+	const int orig_euid = geteuid();
 #endif
+	const int orig_egid = getegid();
 
-#if defined(HAVE_GETEGID) && defined(HAVE_GETGID)
-	if (getegid() == getgid()) {
-	    skip_setgid = 1;
-	}
-#endif
-
-	if (!skip_setgid) {
-	    int setgid_err;
-#ifdef HAVE_SETRESGID
-	    setgid_err = setresgid(getgid(), getgid(), getgid());
-#elif defined(HAVE_SETREGID)
-#if defined(HAVE_GETEGID) && defined(HAVE_SETGID) && defined(HAVE_GETUID)
-	    setgid_err = setregid(getgid(), getgid());
-#else
-	    zwarnnam("unsetopt",
-		"PRIVILEGED: can't drop privileges; setregid available, but cannot check if saved gid changed");
+	/*
+	 * Set the GID first as if we set the UID to non-privileged it
+	 * might be impossible to restore the GID.
+	 */
+	{
+#ifndef HAVE_SETRESGID
+	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; setresgid() and friends not available");
 	    return -1;
-#endif
 #else
-	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; setresgid and setregid not available");
-	    return -1;
-#endif
+	    int setgid_err;
+	    setgid_err = setresgid(getgid(), getgid(), getgid());
 	    if (setgid_err) {
 		zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; failed to change group ID: %e", errno);
 		return -1;
 	    }
+#endif
 	}
 
-	if (!skip_setuid) {
-#if defined(HAVE_GETEUID) && defined(HAVE_SETUID)
-	    int orig_euid = geteuid();
-#endif
+	/* Set the UID second. */
+	{
+#ifndef HAVE_SETRESUID
+	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; setresuid() and friends not available");
+	    return -1;
+#else
 	    int setuid_err;
-#if defined(HAVE_GETEUID) && defined(HAVE_INITGROUPS) && defined(HAVE_GETPWUID)
+
+# ifdef HAVE_INITGROUPS
+	    /* Set the supplementary groups list. */
 	    if (geteuid() == 0) {
 		struct passwd *pw = getpwuid(getuid());
 		if (pw == NULL) {
-		    zwarnnam("unsetopt", "can't drop privileges; failed to get user information for uid %d: %e",
-			    getuid(), errno);
-		    return -1;
-		}
-		if (initgroups(pw->pw_name, pw->pw_gid)) {
+		    zwarnnam("unsetopt", "can't drop privileges; failed to get user information for uid %L: %e",
+			    (long)getuid(), errno);
+		    failed = 1;
+		} else if (initgroups(pw->pw_name, pw->pw_gid)) {
 		    zwarnnam("unsetopt", "can't drop privileges; failed to set supplementary group list: %e", errno);
 		    return -1;
 		}
+	    } else if (getuid() != 0 &&
+		    (geteuid() != getuid() || orig_egid != getegid())) {
+		zwarnnam("unsetopt", "PRIVILEGED: supplementary group list not changed due to lack of permissions: EUID=%L",
+			(long)geteuid());
+		failed = 1;
 	    }
-#endif
+# else
+	    /* initgroups() isn't in POSIX.  If it's not available on the system,
+	     * we silently skip it. */
+# endif
 
-#ifdef HAVE_SETRESUID
 	    setuid_err = setresuid(getuid(), getuid(), getuid());
-#elif defined(HAVE_SETREUID)
-#if defined(HAVE_GETEUID) && defined(HAVE_SETUID) && defined(HAVE_GETUID)
-	    setuid_err = setreuid(getuid(), getuid());
-#else
-	    zwarnnam("unsetopt",
-		"PRIVILEGED: can't drop privileges; setreuid available, but cannot check if saved uid changed");
-	    return -1;
-#endif
-#else
-	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; setresuid and setreuid not available");
-	    return -1;
-#endif
 	    if (setuid_err) {
 		zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; failed to change user ID: %e", errno);
 		return -1;
 	    }
-#if defined(HAVE_GETEUID) && defined(HAVE_SETUID) && defined(HAVE_GETUID)
-	    if (getuid() != 0 && !setuid(orig_euid)) {
-		zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; was able to restore the euid");
-		return -1;
-	    }
 #endif
 	}
 
-#if defined(HAVE_GETEGID) && defined(HAVE_SETGID) && defined(HAVE_GETUID)
-	if (getuid() != 0 && !skip_setgid && !setgid(orig_egid)) {
+#ifdef HAVE_SETGID
+	if (getuid() != 0 && orig_egid != getegid() &&
+		(setgid(orig_egid) != -1 || setegid(orig_egid) != -1)) {
 	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; was able to restore the egid");
 	    return -1;
 	}
 #endif
 
+#ifdef HAVE_SETUID
+	if (getuid() != 0 && orig_euid != geteuid() &&
+		(setuid(orig_euid) != -1 || seteuid(orig_euid) != -1)) {
+	    zwarnnam("unsetopt", "PRIVILEGED: can't drop privileges; was able to restore the euid");
+	    return -1;
+	}
+#endif
+
+	if (failed) {
+	    /* A warning message has been printed. */
+	    return -1;
+	}
+
 #ifdef JOB_CONTROL
     } else if (!force && optno == MONITOR && value) {
 	if (new_opts[optno] == value)
diff --git a/Src/zsh.mdd b/Src/zsh.mdd
index 3e5788af5..9bcaccae5 100644
--- a/Src/zsh.mdd
+++ b/Src/zsh.mdd
@@ -13,7 +13,8 @@ objects="builtin.o compat.o cond.o context.o \
 exec.o glob.o hashtable.o hashnameddir.o \
 hist.o init.o input.o jobs.o lex.o linklist.o loop.o math.o \
 mem.o module.o options.o params.o parse.o pattern.o prompt.o signals.o \
-signames.o sort.o string.o subst.o text.o utils.o watch.o"
+signames.o sort.o string.o subst.o text.o utils.o watch.o \
+openssh_bsd_setres_id.o"
 
 headers="../config.h zsh_system.h zsh.h sigcount.h signals.h \
 prototypes.h hashtable.h ztype.h"
diff --git a/Src/zsh_system.h b/Src/zsh_system.h
index 85e198f2e..161b073b4 100644
--- a/Src/zsh_system.h
+++ b/Src/zsh_system.h
@@ -468,30 +468,90 @@ struct timespec {
 # define setpgrp setpgid
 #endif
 
-/* can we set the user/group id of a process */
+/* compatibility wrappers */
 
-#ifndef HAVE_SETUID
+/* Our strategy is as follows:
+ *
+ * - Ensure that either setre[ug]id() or set{e,}[ug]id() is available.
+ * - If setres[ug]id() are missing, provide them in terms of either
+ *   setre[ug]id() or set{e,}[ug]id(), whichever is available.
+ * - Provide replacement setre[ug]id() or set{e,}[ug]id() if they are not
+ *   available natively.
+ *
+ * There isn't a circular dependency because, right off the bat, we check that
+ * there's an end condition, and #error out otherwise.
+ */
+#if !defined(HAVE_SETREUID) && !(defined(HAVE_SETEUID) && defined(HAVE_SETUID))
+  /*
+   * If you run into this error, you have two options:
+   * - Teach zsh how to do the equivalent of setreuid() on your system
+   * - Remove support for PRIVILEGED option, and then remove the #error.
+   */
+# error "Don't know how to change UID"
+#endif
+#if !defined(HAVE_SETREGID) && !(defined(HAVE_SETEGID) && defined(HAVE_SETGID))
+  /* See above comment. */
+# error "Don't know how to change GID"
+#endif
+
+/* Provide setresuid(). */
+#ifndef HAVE_SETRESUID
+int	setresuid(uid_t, uid_t, uid_t);
+# define HAVE_SETRESUID
+# define ZSH_IMPLEMENT_SETRESUID
 # ifdef HAVE_SETREUID
-#  define setuid(X) setreuid(X,X)
-#  define setgid(X) setregid(X,X)
-#  define HAVE_SETUID
+#  define ZSH_HAVE_NATIVE_SETREUID
 # endif
 #endif
 
-/* can we set the effective user/group id of a process */
+/* Provide setresgid(). */
+#ifndef HAVE_SETRESGID
+int	setresgid(gid_t, gid_t, gid_t);
+# define HAVE_SETRESGID
+# define ZSH_IMPLEMENT_SETRESGID
+# ifdef HAVE_SETREGID
+#  define ZSH_HAVE_NATIVE_SETREGID
+# endif
+#endif
 
+/* Provide setreuid(). */
+#ifndef HAVE_SETREUID
+# define setreuid(X, Y) setresuid((X), (Y), -1)
+# define HAVE_SETREUID
+#endif
+
+/* Provide setregid(). */
+#ifndef HAVE_SETREGID
+# define setregid(X, Y) setresgid((X), (Y), -1)
+# define HAVE_SETREGID
+#endif
+
+/* Provide setuid(). */
+/* ### TODO: Either remove this (this function has been standard since 1985),
+ * ###       or rewrite this without multiply-evaluating the argument */
+#ifndef HAVE_SETUID
+# define setuid(X) setreuid((X), (X))
+# define HAVE_SETUID
+#endif
+
+/* Provide setgid(). */
+#ifndef HAVE_SETGID
+/* ### TODO: Either remove this (this function has been standard since 1985),
+ * ###       or rewrite this without multiply-evaluating the argument */
+#  define setgid(X) setregid((X), (X))
+#  define HAVE_SETGID
+#endif
+
+/* Provide seteuid(). */
 #ifndef HAVE_SETEUID
-# ifdef HAVE_SETREUID
-#  define seteuid(X) setreuid(-1,X)
-#  define setegid(X) setregid(-1,X)
-#  define HAVE_SETEUID
-# else
-#  ifdef HAVE_SETRESUID
-#   define seteuid(X) setresuid(-1,X,-1)
-#   define setegid(X) setresgid(-1,X,-1)
-#   define HAVE_SETEUID
-#  endif
-# endif
+# define seteuid(X) setreuid(-1, (X))
+# define HAVE_SETEUID
+#endif
+
+/* Provide setegid(). */
+#ifndef HAVE_SETEGID
+# define setegid(X) setregid(-1, (X))
+# define HAVE_SETEGID
 #endif
 
 #ifdef HAVE_SYS_RESOURCE_H
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index c4b101bdb..680f49082 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -1391,3 +1391,18 @@ F:Regression test for workers/41811
 ?(anon):4: `break' active at end of function scope
 ?(anon):4: `break' active at end of function scope
 ?(anon):4: `break' active at end of function scope
+
+# There are further tests for PRIVILEGED in P01privileged.ztst.
+ if [[ -o privileged ]]; then
+   unsetopt privileged
+ fi
+ unsetopt privileged
+0:PRIVILEGED sanity check: unsetting is idempotent
+F:If this test fails at the first unsetopt, refer to P01privileged.ztst.
+
+  if [[ -o privileged ]]; then
+    (( UID != EUID ))
+  else
+    (( UID == EUID ))
+  fi
+0:PRIVILEGED sanity check: default value is correct
diff --git a/configure.ac b/configure.ac
index 2474c270e..f2d65ecfc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1310,9 +1310,8 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
 	       inet_aton inet_pton inet_ntop \
 	       getlogin getpwent getpwnam getpwuid getgrgid getgrnam \
 	       initgroups nis_list \
-	       getuid setuid seteuid setreuid setresuid setsid \
-	       getgid setgid setegid setregid setresgid \
-	       geteuid getegid \
+	       setuid seteuid setreuid setresuid setsid \
+	       setgid setegid setregid setresgid \
 	       memcpy memmove strstr strerror strtoul \
 	       getrlimit getrusage \
 	       setlocale \