fix several security vulnerabilities found by iDEFENSE in fcronsighup

diff --git a/Makefile.in b/Makefile.in
index 1ea2961459b9b8719dfcb3eb7a457371165da6f2..7049c7c4424390d4150e5cdf55812d75f434ae4e 100644 (file)
--- a/Makefile.in
+++ b/Makefile.in
@@ -4,7 +4,7 @@
 
 # @configure_input@
 
-# $Id: Makefile.in,v 1.103 2003-12-29 18:26:07 thib Exp $
+# $Id: Makefile.in,v 1.104 2004-11-13 19:43:08 thib Exp $
 
 # The following should not be edited manually (use configure options)
 # If you must do it, BEWARE : some of the following is also defined
@@ -77,8 +77,10 @@ RCSNOCI:=.*\(.html\|VERSION\|MANIFEST\|configure\|install.sh\|config.log\|config
 RUN_NON_PRIVILEGED := @RUN_NON_PRIVILEGED@
 ifeq ($(RUN_NON_PRIVILEGED), 1)
        BINMODE:=111
+       BINMODESIGHUP:=111
 else
        BINMODE:=6111
+       BINMODESIGHUP:=4110
 endif
 
 ifeq ($(FCRONDYN), 1)
@@ -121,7 +123,7 @@ install: all
 
        $(INSTALL) -g $(ROOTGROUP) -o $(ROOTNAME) -m 110 -s fcron $(DESTSBIN)
        $(INSTALL) -g $(GROUPNAME) -o $(USERNAME) -m $(BINMODE) -s fcrontab $(DESTBIN)
-       $(INSTALL) -g $(ROOTGROUP) -o $(ROOTNAME) -m $(BINMODE) -s fcronsighup $(DESTBIN)
+       $(INSTALL) -g $(GROUPNAME) -o $(ROOTNAME) -m $(BINMODESIGHUP) -s fcronsighup $(DESTBIN)
 ifeq ($(FCRONDYN), 1)
        $(INSTALL) -g $(GROUPNAME) -o $(USERNAME) -m $(BINMODE) -s fcrondyn $(DESTBIN)
 endif

diff --git a/configure.in b/configure.in
index d3ade90d3251d569f9640bf8f989017c3de62040..24f2c59d4d417c6dd5353a4bee2bdcf468735e5a 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -13,7 +13,7 @@ AC_PREFIX_DEFAULT($prefix)
 AC_CONFIG_HEADER(config.h)
 AC_PREREQ(2.57)
 
-vers="2.9.5"
+vers="2.9.5.1"
 vers_quoted="\"$vers\""
 AC_DEFINE_UNQUOTED(VERSION, $vers)
 AC_DEFINE_UNQUOTED(VERSION_QUOTED, $vers_quoted)
@@ -44,6 +44,7 @@ AC_CHECK_HEADERS(strings.h)
 AC_CHECK_HEADERS(sys/types.h sys/socket.h sys/un.h)
 AC_CHECK_HEADERS(security/pam_appl.h pam/pam_appl.h crypt.h shadow.h)
 AC_CHECK_HEADERS(sys/resource.h)
+AC_CHECK_HEADERS(grp.h)
 
 dnl Checks for typedefs, structures, and compiler characteristics.
 AC_C_CONST

diff --git a/fcronsighup.c b/fcronsighup.c
index 8b00c4057150c54512cd1043b6afec51e2dfd2c2..ba65fbeabe4effd043f16c66a3d14417b0f421ce 100644 (file)
--- a/fcronsighup.c
+++ b/fcronsighup.c
@@ -21,19 +21,21 @@
  *  `LICENSE' that comes with the fcron source distribution.
  */
 
- /* $Id: fcronsighup.c,v 1.7 2003-12-25 22:39:27 thib Exp $ */
+ /* $Id: fcronsighup.c,v 1.8 2004-11-13 19:41:55 thib Exp $ */
 
 #include "fcronsighup.h"
 #include "global.h"
 #include "allow.h"
 
-char rcs_info[] = "$Id: fcronsighup.c,v 1.7 2003-12-25 22:39:27 thib Exp $";
+char rcs_info[] = "$Id: fcronsighup.c,v 1.8 2004-11-13 19:41:55 thib Exp $";
 
 void usage(void);
 void sig_daemon(void);
 pid_t read_pid(void);
 
 uid_t uid = 0;
+uid_t fcrontab_uid = 0;
+
 
 #ifdef DEBUG
 char debug_opt = 1;       /* set to 1 if we are in debug mode */
@@ -124,6 +126,11 @@ sig_daemon(void)
        fprintf(stderr, "Modifications will be taken into account"
                " at %s.\n", buf);
 
+       /* if fcrontabs is too long, snprintf will not be able to add "/fcrontab.sig"
+        * string at the end of sigfile */
+       if ( strlen(fcrontabs) > (sizeof(sigfile) - sizeof("/fcrontab.sig")) )
+           die("fcrontabs string too long (more than %d characters)",
+                 (sizeof(sigfile) - sizeof("/fcrontab.sig")));
        snprintf(sigfile, sizeof(sigfile), "%s/fcrontab.sig", fcrontabs);
 
        switch ( fork() ) {
@@ -178,9 +185,20 @@ sig_daemon(void)
 
     foreground = 1;
 
+#ifdef USE_SETE_ID
+    if (seteuid(ROOTUID) != 0)
+       error_e("seteuid(ROOTUID)");
+#endif /* USE_SETE_ID */
+
     if ( kill(daemon_pid, SIGHUP) != 0)
        die_e("could not send SIGHUP to daemon (pid %d)", daemon_pid);
 
+#ifdef USE_SETE_ID
+    /* get user's permissions */
+    if (seteuid(fcrontab_uid) != 0) 
+       die_e("Could not change euid to " USERNAME "[%d]", uid); 
+#endif /* USE_SETE_ID */
+
 }
 
 
@@ -192,13 +210,28 @@ main(int argc, char **argv)
     if (strrchr(argv[0],'/')==NULL) prog_name = argv[0];
     else prog_name = strrchr(argv[0],'/')+1;
 
+    if ( ! (pass = getpwnam(USERNAME)) )
+       die("user \"%s\" is not in passwd file. Aborting.", USERNAME);
+    fcrontab_uid = pass->pw_uid;
+
+#ifdef USE_SETE_ID
+    /* get user's permissions */
+    if (seteuid(fcrontab_uid) != 0) 
+       die_e("Could not change euid to " USERNAME "[%d]", uid); 
+#endif /* USE_SETE_ID */
+
     if ( argc == 2 )
        fcronconf = argv[1];
     else if (argc > 2 )
        usage();
 
     /* read fcron.conf and update global parameters */
+    /* We deactivate output to console, because otherwise it may be used
+     * by a malicious user to read some data it is not allow to read
+     * (fcronsighup is suid root) */
+    foreground = 0;
     read_conf();
+    foreground = 1;
     
     uid = getuid();
 

diff --git a/global.h b/global.h
index 74b707c721f180e19ce18b686084ccc7a9b21f9f..9f0393ee76493bb31a6036a6f15da1ca9340656a 100644 (file)
--- a/global.h
+++ b/global.h
@@ -21,7 +21,7 @@
  *  `LICENSE' that comes with the fcron source distribution.
  */
 
- /* $Id: global.h,v 1.42 2004-01-29 10:30:12 thib Exp $ */
+ /* $Id: global.h,v 1.43 2004-11-13 19:42:57 thib Exp $ */
 
 
 /* 
@@ -54,6 +54,10 @@
 #include <getopt.h>
 #endif
 
+#ifdef HAVE_GRP_H
+#include <grp.h>
+#endif
+
 #ifdef HAVE_LIMITS_H
 #include <limits.h>
 #endif

diff --git a/subs.c b/subs.c
index 849d066c926c4f27bc75c447fa10673b35dbe727..223a2c7220cebca4fac8656cfb1904800e2e0559 100644 (file)
--- a/subs.c
+++ b/subs.c
@@ -22,7 +22,7 @@
  *  `LICENSE' that comes with the fcron source distribution.
  */
 
- /* $Id: subs.c,v 1.21 2004-04-29 19:34:45 thib Exp $ */
+ /* $Id: subs.c,v 1.22 2004-11-13 19:43:36 thib Exp $ */
 
 #include "global.h"
 #include "subs.h"
@@ -146,6 +146,8 @@ read_conf(void)
     char *ptr1 = NULL, *ptr2 = NULL;
     short namesize = 0;
     char err_on_enoent = 0;
+    struct group *gr = NULL;
+    gid_t fcrongid = -1;
 
     if (fcronconf != NULL)
        /* fcronconf has been set by -c option : file must exist */
@@ -167,11 +169,20 @@ read_conf(void)
        }
     }
 
-    /* check if the file is secure : owned and writable only by root */
-    if ( fstat(fileno(f), &st) != 0 || st.st_uid != ROOTUID
+    /* get fcrongid */
+    gr = getgrnam(GROUPNAME);
+    if ( gr == NULL ) {
+       die_e("Unable to find %s in /etc/group", GROUPNAME);
+    }
+    fcrongid = gr->gr_gid;
+
+    /* check if the file is secure : owner:root, group:fcron,
+     * writable only by owner */
+    if ( fstat(fileno(f), &st) != 0 
+        || st.st_uid != ROOTUID || st.st_gid != fcrongid
         || st.st_mode & S_IWGRP || st.st_mode & S_IWOTH ) {
-       error("Conf file (%s) must be owned by root and (no more than) 644 : "
-             "ignored", fcronconf);
+       error("Conf file (%s) must be owned by root:" GROUPNAME 
+             " and (no more than) 644 : ignored", fcronconf, GROUPNAME);
        fclose(f);
        return;
     }