From: Thibault Godouet <yo8192@users.noreply.github.com>
Date: Wed, 24 Feb 2010 23:40:59 +0000 (+0000)
Subject: Improved fcrontab security fix which allowed to read file readable by the group fcron
X-Git-Tag: ver3_0_5~7
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7ef990011a3493bed51094d9919d838f4386a9cf;p=thirdparty%2Ffcron.git

Improved fcrontab security fix which allowed to read file readable by the group fcron
---

diff --git a/fcrontab.c b/fcrontab.c
index e6693d9..c6deb05 100644
--- a/fcrontab.c
+++ b/fcrontab.c
@@ -75,6 +75,7 @@ char debug_opt = 0;       /* set to 1 if we are in debug mode */
 char *user = NULL;
 char *runas = NULL;
 uid_t uid = 0;
+gid_t gid = 0;
 uid_t asuid = 0;
 gid_t asgid = 0;
 uid_t fcrontab_uid = 0;
@@ -181,6 +182,30 @@ xexit(int exit_val)
 
 }
 
+int
+open_as_user(const char *pathname, int flags, uid_t openuid, gid_t opengid)
+/* Become user and call open(), then revert back to who we were */
+{
+    uid_t orig_euid = geteuid();
+    gid_t orig_egid = getegid();
+    int fd = -1;
+
+    if (seteuid(openuid) < 0)
+        die_e("open_as_user(): could not change euid to %d", uid);
+    if (setegid(opengid) < 0)
+        die_e("open_as_user(): could not change egid to %d", gid);
+
+    fd = open(pathname, flags);
+
+    /* change the effective uid/gid back to original values */
+    if (seteuid(orig_euid) < 0)
+        die_e("open_as_user(): could not revert to euid %d", orig_euid);
+    if (setegid(orig_egid) < 0)
+        die_e("open_as_user(): could not revert to egid %d", orig_egid);
+
+    return fd;
+    
+}
 
 int
 copy_src(int from, char *dest)
@@ -473,10 +498,14 @@ edit_file(char *fcron_orig)
 	}
 	fclose(f);
 	f = NULL;
+
+        if ( ferror(fi) )
+            error_e("Error while writing new fcrontab to %s");
     }
 
     /* Don't close fi, because we still need the file descriptor 'file' */
-    fflush(fi);
+    if ( fflush(fi) != 0 )
+        die_e("Could not fflush(%s)", fi);
     fi = NULL;
 
     do {
@@ -635,7 +664,8 @@ install_stdin(void)
     debug("Copied stdin to %s, about to parse file %s...", tmp_str, tmp_str); 
 
     /* don't closes tmp_fd as it will be used for make_file(): */
-    fflush(tmp_file);
+    if ( fflush(tmp_file) != 0 )
+        die_e("Could not fflush(%s)", tmp_file);
 
     if ( make_file(tmp_str, tmp_fd) == ERR )
 	goto exiterr;
@@ -980,6 +1010,7 @@ main(int argc, char **argv)
     else prog_name = strrchr(argv[0],'/')+1;
     
     uid = getuid();
+    gid = getgid();
 
     errno = 0;
     if ( ! (pass = getpwnam(USERNAME)) )
@@ -1099,7 +1130,7 @@ main(int argc, char **argv)
 		file[sizeof(file)-1] = '\0';
 	    }
 
-            fd = open(file, O_RDONLY);
+            fd = open_as_user(file, O_RDONLY, uid, gid);
             if (fd < 0)
                 die_e("Could not open file %s", file);
 	    if (make_file(file, fd) == OK)
diff --git a/fileconf.c b/fileconf.c
index 50e087d..6258c12 100644
--- a/fileconf.c
+++ b/fileconf.c
@@ -265,7 +265,8 @@ read_file(char *filename, int fd)
     file_base = cf;
 
     /* don't close as underlying fd may still be used by calling function */
-    fflush(file);
+    if (fflush(file) != 0)
+        error_e("could not fflush() file_name");
     
     free(default_line.cl_runas);
     free(default_line.cl_mailto);