Bug 4111: leave_suid() does not properly handle error codes returned by setuid

... this will cause privilege escalation in the rare case that setuid fails.
So far there are no known cases of this happening when downgrading from root.

Also fixes several incorrect uses of errno which may have been obscuring
error message details if it did happen.

diff --git a/src/tools.cc b/src/tools.cc
index 73c9842e9f8655c86a60fb3f2320c008d2f98f13..a21d7f2e2f9ab785a24a88e790b1110422965aa8 100644 (file)
--- a/src/tools.cc
+++ b/src/tools.cc
@@ -534,19 +534,22 @@ leave_suid(void)
     }
 
 #if HAVE_SETRESUID
-
-    if (setresuid(Config2.effectiveUserID, Config2.effectiveUserID, 0) < 0)
-        debugs(50, DBG_CRITICAL, "ALERT: setresuid: " << xstrerror());
+    if (setresuid(Config2.effectiveUserID, Config2.effectiveUserID, 0) < 0) {
+        const auto xerrno = errno;
+        fatalf("FATAL: setresuid: %s", xstrerr(xerrno));
+    }
 
 #elif HAVE_SETEUID
-
-    if (seteuid(Config2.effectiveUserID) < 0)
-        debugs(50, DBG_CRITICAL, "ALERT: seteuid: " << xstrerror());
+    if (seteuid(Config2.effectiveUserID) < 0) {
+        const auto xerrno = errno;
+        fatalf("FATAL: seteuid: %s", xstrerr(xerrno));
+    }
 
 #else
-
-    if (setuid(Config2.effectiveUserID) < 0)
-        debugs(50, DBG_CRITICAL, "ALERT: setuid: " << xstrerror());
+    if (setuid(Config2.effectiveUserID) < 0) {
+        const auto xerrno = errno;
+        fatalf("FATAL: setuid: %s", xstrerr(xerrno));
+    }
 
 #endif
 
@@ -566,8 +569,10 @@ enter_suid(void)
 {
     debugs(21, 3, "enter_suid: PID " << getpid() << " taking root privileges");
 #if HAVE_SETRESUID
-    if (setresuid((uid_t)-1, 0, (uid_t)-1) < 0)
-        debugs (21, 3, "enter_suid: setresuid failed: " << xstrerror ());
+    if (setresuid((uid_t)-1, 0, (uid_t)-1) < 0) {
+        const auto xerrno = errno;
+        debugs (21, 3, "enter_suid: setresuid failed: " << xstrerr(xerrno));
+    }
 #else
 
     setuid(0);