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

[INN Commit]

    Date: Thursday, May 14, 2015 @ 06:12:36
  Author: iulius
Revision: 9854

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:
  trunk/frontends/rnews.c
  trunk/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-08 14:34:46 UTC (rev 9853)
+++ frontends/rnews.c	2015-05-14 13:12:36 UTC (rev 9854)
@@ -857,10 +857,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-08 14:34:46 UTC (rev 9853)
+++ lib/newsuser.c	2015-05-14 13:12:36 UTC (rev 9854)
@@ -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);