[squid-dev] [PATCH] Bug 4111 fix

[Amos Jeffries]

This bug had unfortunately gotten lost in the flood. The bug reporters
proposed patch seems to be correct in terms of its intention.

This version of the patch adds the missing {} and uses fatalf() instead
of exit(). It also goes a little further and corrects the outstanding
errno handling issues in the affected functions. Both of these
diffferences are to ensure consistent error reporting on the abort.

Amos
-------------- next part --------------
=== modified file 'src/tools.cc'
--- src/tools.cc	2016-01-24 17:41:43 +0000
+++ src/tools.cc	2016-02-16 22:48:57 +0000
@@ -534,19 +534,22 @@
     }
 
 #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) {
+        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) {
+        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) {
+        auto xerrno = errno;
+        fatalf("FATAL: setuid: %s", xstrerr(xerrno));
+    }
 
 #endif
 
@@ -566,8 +569,10 @@
 {
     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) {
+        auto xerrno = errno;
+        debugs (21, 3, "enter_suid: setresuid failed: " << xstrerr(xerrno));
+    }
 #else
 
     setuid(0);
@@ -581,8 +586,9 @@
 #endif
 }
 
-/* Give up the posibility to gain privilegies.
- * this should be used before starting a sub process
+/**
+ * Give up the posibility to gain privileges.
+ * This should be used before starting a sub process.
  */
 void
 no_suid(void)
@@ -592,11 +598,15 @@
     uid = geteuid();
     debugs(21, 3, "no_suid: PID " << getpid() << " giving up root priveleges forever");
 
-    if (setuid(0) < 0)
-        debugs(50, DBG_IMPORTANT, "WARNING: no_suid: setuid(0): " << xstrerror());
+    if (setuid(0) < 0) {
+        auto xerrno = errno;
+        debugs(50, DBG_IMPORTANT, "WARNING: no_suid: setuid(0): " << xstrerr(xerrno));
+    }
 
-    if (setuid(uid) < 0)
-        debugs(50, DBG_IMPORTANT, "ERROR: no_suid: setuid(" << uid << "): " << xstrerror());
+    if (setuid(uid) < 0) {
+        auto xerrno = errno;
+        debugs(50, DBG_IMPORTANT, "ERROR: no_suid: setuid(" << uid << "): " << xstrerr(xerrno));
+    }
 
     restoreCapabilities(false);