From: thib <thib>
Date: Sat, 13 Nov 2004 19:41:55 +0000 (+0000)
Subject: fix several security vulnerabilities found by iDEFENSE in fcronsighup
X-Git-Tag: ver2_9_5_1~11
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=14e2afd8656c952bc8b1d09bd424cd7555cd772d;p=fcron

fix several security vulnerabilities found by iDEFENSE in fcronsighup
---

diff --git a/Makefile.in b/Makefile.in
index 1ea2961..7049c7c 100644
--- 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 d3ade90..24f2c59 100644
--- 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 8b00c40..ba65fbe 100644
--- 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 74b707c..9f0393e 100644
--- 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 849d066..223a2c7 100644
--- 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;
     }