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
=== 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);
 

_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to