From f37c181b29149c6829cc8a86f7348335a08d9670 Mon Sep 17 00:00:00 2001 From: Peter Stephenson Date: Fri, 15 Jun 2018 10:27:29 +0100 Subject: 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. --- Src/options.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'Src') 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; -- cgit 1.4.1