INN commit: branches/2.5 (frontends/rnews.c lib/newsuser.c)

INN Commit rra at isc.org
Thu May 14 13:13:11 UTC 2015


    Date: Thursday, May 14, 2015 @ 06:13:11
  Author: iulius
Revision: 9855

Verify that setuid() and setgid() actually succeed

See:
  https://lwn.net/Articles/451985/
for a discussion of the issues in this area.

The checks in newuser.c are probably unnecessary due to the subsequent
tests.  rnews.c is straight-up broken though.

Thanks to Richard Kettlewell for the patch.

Modified:
  branches/2.5/frontends/rnews.c
  branches/2.5/lib/newsuser.c

-------------------+
 frontends/rnews.c |    5 +----
 lib/newsuser.c    |    8 ++++++--
 2 files changed, 7 insertions(+), 6 deletions(-)

Modified: frontends/rnews.c
===================================================================
--- frontends/rnews.c	2015-05-14 13:12:36 UTC (rev 9854)
+++ frontends/rnews.c	2015-05-14 13:13:11 UTC (rev 9855)
@@ -855,10 +855,7 @@
        other setups where rnews might be setuid news or be run by other
        processes in the news group. */
     if (getuid() == 0 || geteuid() == 0) {
-        uid_t uid;
-
-        get_news_uid_gid(&uid, false, true);
-        setuid(uid);
+        ensure_news_user(true);
     }
 
     if (!innconf_read(NULL))

Modified: lib/newsuser.c
===================================================================
--- lib/newsuser.c	2015-05-14 13:12:36 UTC (rev 9854)
+++ lib/newsuser.c	2015-05-14 13:13:11 UTC (rev 9855)
@@ -71,7 +71,9 @@
             /* NB:  mustn't be run as root, unless "may_setuid" is true. */
             die("must be run as %s, not as root", innconf->runasuser);
         }
-        setuid(uid);
+        if (setuid(uid) < 0) {
+            sysdie("failed to setuid");
+        }
     }
     if (geteuid() != uid || getuid() != uid) {
         die("must be run as %s", innconf->runasuser);
@@ -88,7 +90,9 @@
 
     get_news_uid_gid(false, &gid, true);
     if (may_setgid && geteuid() == 0) {
-        setgid(gid);
+        if (setgid(gid) < 0) {
+            sysdie("failed to setgid");
+        }
     }
     if (getegid() != gid || getgid() != gid) {
         die ("must be run as %s group", innconf->runasgroup);



More information about the inn-committers mailing list