[torquedev] more issues with recent security fix

Garrick Staples garrick at clusterresources.com
Mon Oct 23 16:46:56 MDT 2006


Turns out we aren't "there" yet.

In 2.1.5, TM is broken with single node jobs, and jobs fail to rerun.

Also, I found some similar security problems with the spool handling
with rerunning jobs.

Here is another patch that hopefully buttons everything up.  I'm going
to wait a few days before the next release.

Comments?

-------------- next part --------------
Index: src/resmom/start_exec.c
===================================================================
--- src/resmom/start_exec.c	(revision 1066)
+++ src/resmom/start_exec.c	(working copy)
@@ -4838,19 +4838,53 @@
   int   keeping;
   char *path;
   int   old_umask;
+  struct stat statbuf;
 
   if ((path = std_file_name(pjob,which,&keeping)) == NULL)
     {
     return(-1);
     }
 
+  /* these checks are a bit complicated.  If keeping, we do what the user
+   * says.  Otherwise, make sure we aren't following a symlink and that
+     the user owns the file without breaking /dev/null. */
+
   if (keeping)
     {
     mode &= ~O_EXCL; 
     }
+  else
+    {
+    if (lstat(path,&statbuf) == 0)
+      {
+      if (S_ISLNK(statbuf.st_mode))
+        {
+        log_err(-1,"open_std_file","std file is symlink, someone is doing something fishy");
 
-    /* in user's home,  may be NFS mounted, must create as user */
+        return(-1);
+        }
 
+      if (S_ISREG(statbuf.st_mode))
+        {
+        if (statbuf.st_uid != pjob->ji_qs.ji_un.ji_momt.ji_exuid)
+          {
+          log_err(-1,"open_std_file","std file exists with the wrong owner, someone is doing something fishy");
+
+          return(-1);
+          }
+        if (statbuf.st_gid != exgid)
+          {
+          log_err(-1,"open_std_file","std file exists with the wrong group, someone is doing something fishy");
+
+          return(-1);
+          }
+        }
+
+      /* seems reasonably safe to append to the existing file */
+      mode &= ~O_EXCL; 
+      }
+    }
+
 #if defined(HAVE_SETEUID) && defined(HAVE_SETEGID)
 
     /* most systems */
Index: src/server/req_quejob.c
===================================================================
--- src/server/req_quejob.c	(revision 1059)
+++ src/server/req_quejob.c	(working copy)
@@ -1310,7 +1310,10 @@
     }
 
   if (preq->rq_ind.rq_jobfile.rq_sequence == 0)
-    fds = open(namebuf,O_WRONLY|O_CREAT|O_TRUNC|O_Sync,0600);
+    {
+    unlink(namebuf);
+    fds = open(namebuf,O_WRONLY|O_CREAT|O_EXCL|O_Sync,0600);
+    }
   else
     fds = open(namebuf,O_WRONLY|O_APPEND|O_Sync,0600);
 


More information about the torquedev mailing list