dspam-dev  

Re: [dspam-dev] cssclean is a redheaded stepchild

Frank Cusack
Sat, 14 Apr 2007 21:05:01 -0700

updated for 3.8.0

On March 6, 2007 11:14:42 PM -0800 Frank Cusack <[EMAIL PROTECTED]> wrote:
improved patch.  cleans up newfile on error.

On March 5, 2007 11:38:02 PM -0800 Frank Cusack <[EMAIL PROTECTED]>
wrote:
and here is a beating for it.  the other css* tools need similar fixes.

- fix symlink vulnerability when opening new css file (this has also been
  fixed in other patches available on the net), also fixes rename()
problem
  across filesystems
- lock the database when operating on it (!!!)
- set correct file owner/group/mode for cleaned css file

-frank






diff -urp dspam-3.8.0.orig/src/hash_drv.c dspam-3.8.0/src/hash_drv.c
--- dspam-3.8.0.orig/src/hash_drv.c     2006-05-27 14:00:36.000000000 -0700
+++ dspam-3.8.0/src/hash_drv.c  2007-04-14 19:56:31.885824232 -0700
@@ -251,14 +251,14 @@ dspam_shutdown_driver (DRIVER_CTX *DTX)
 
 int
 _hash_drv_lock_get (
-  DSPAM_CTX *CTX,
+  const char *home,
   struct _hash_drv_storage *s,
   const char *username)
 {
   char filename[MAX_FILENAME_LENGTH];
   int r;
 
-  _ds_userdir_path(filename, CTX->home, username, "lock");
+  _ds_userdir_path(filename, home, username, "lock");
   _ds_prepare_path_for(filename);
 
   s->lock = fopen(filename, "a");
@@ -490,7 +490,7 @@ _ds_init_storage (DSPAM_CTX * CTX, void 
     else
       _ds_userdir_path(db, CTX->home, CTX->group, "css");
 
-    lock_result = _hash_drv_lock_get (CTX, s, 
+    lock_result = _hash_drv_lock_get (CTX->home, s, 
       (CTX->group) ? CTX->group : CTX->username);
     if (lock_result < 0) 
       goto BAIL;
diff -urp dspam-3.8.0.orig/src/hash_drv.h dspam-3.8.0/src/hash_drv.h
--- dspam-3.8.0.orig/src/hash_drv.h     2006-05-27 14:00:36.000000000 -0700
+++ dspam-3.8.0/src/hash_drv.h  2007-04-14 19:56:31.887814972 -0700
@@ -88,7 +88,7 @@ int _hash_drv_set_spamtotals
   (DSPAM_CTX * CTX);
 
 int _hash_drv_lock_get (
-  DSPAM_CTX * CTX, 
+  const char *home, 
   struct _hash_drv_storage *s, 
   const char *username);
 
diff -urp dspam-3.8.0.orig/src/tools.hash_drv/cssclean.c 
dspam-3.8.0/src/tools.hash_drv/cssclean.c
--- dspam-3.8.0.orig/src/tools.hash_drv/cssclean.c      2006-05-27 
14:00:36.000000000 -0700
+++ dspam-3.8.0/src/tools.hash_drv/cssclean.c   2007-04-14 20:07:39.445471748 
-0700
@@ -96,11 +96,15 @@ int main(int argc, char *argv[]) {
 int cssclean(const char *filename) {
   int i;
   hash_drv_header_t header;
+  struct _hash_drv_storage s;
   void *offset;
   struct _hash_drv_map old, new;
   hash_drv_spam_record_t rec;
   unsigned long filepos;
-  char newfile[128];
+  char newfile[PATH_MAX];
+  char *oldfile;
+  char *home, *username;
+  struct stat st;
 
   unsigned long hash_rec_max = HASH_REC_MAX;
   unsigned long max_seek     = HASH_SEEK_MAX;
@@ -108,6 +112,7 @@ int cssclean(const char *filename) {
   unsigned long extent_size  = HASH_EXTENT_MAX;
   int pctincrease = 0;
   int flags = 0;
+  int rc = EFAILURE;
 
   if (READ_ATTRIB("HashRecMax"))
     hash_rec_max = strtol(READ_ATTRIB("HashRecMax"), NULL, 0);
@@ -132,19 +137,51 @@ int cssclean(const char *filename) {
   if (READ_ATTRIB("HashMaxSeek"))
      max_seek = strtol(READ_ATTRIB("HashMaxSeek"), NULL, 0);
 
-  snprintf(newfile, sizeof(newfile), "/tmp/%u.css", (unsigned int) getpid());
+  oldfile = strdup(filename);
+  if (!oldfile)
+    return EFAILURE;
+  oldfile = dirname(oldfile);          /* oldfile may not be free'd now */
+  snprintf(newfile, sizeof(newfile), "%s/%u.css", oldfile, (unsigned int) 
getpid());
+
+  /* set home and username */
+  username = strdup(oldfile);
+  home = strdup(oldfile);
+  if (!username || !home)
+    return EFAILURE;
+  username = basename(username);       /* username may not be free'd now */
+  home = dirname(home);                        /* home may not be free'd now */
+  /* make sure at least /two/dirs/to.css for sane operation */
+  if (!strcmp(username, home))
+    return EFAILURE;
+
+  if (stat(filename, &st) < 0)
+    return EFAILURE;
+
+  if (_hash_drv_lock_get (home, &s, username) < 0)
+    return EFAILURE;
 
   if (_hash_drv_open(filename, &old, 0, max_seek,
                      max_extents, extent_size, pctincrease, flags))
-  {
-    return EFAILURE;
-  }
+    goto unlock;
 
   if (_hash_drv_open(newfile, &new, hash_rec_max, max_seek,
-                     max_extents, extent_size, pctincrease, flags))
-  {
+                     max_extents, extent_size, pctincrease, flags)) {
     _hash_drv_close(&old);
-    return EFAILURE;
+    goto unlock;
+  }
+
+  if (fchown(new.fd, st.st_uid, st.st_gid) < 0) {
+    _hash_drv_close(&new);
+    _hash_drv_close(&old);
+    unlink(newfile);
+    goto unlock;
+  }
+
+  if (fchmod(new.fd, st.st_mode) < 0) {
+    _hash_drv_close(&new);
+    _hash_drv_close(&old);
+    unlink(newfile);
+    goto unlock;
   }
 
   filepos = sizeof(struct _hash_drv_header);
@@ -158,7 +195,7 @@ int cssclean(const char *filename) {
           _hash_drv_close(&new);
           _hash_drv_close(&old);
           unlink(newfile);
-          return EFAILURE;
+          goto unlock;
         }
       }
       filepos += sizeof(struct _hash_drv_spam_record);
@@ -170,7 +207,12 @@ int cssclean(const char *filename) {
 
   _hash_drv_close(&new);
   _hash_drv_close(&old);
-  rename(newfile, filename);
-  return 0;
+  if (rename(newfile, filename) < 0)
+    goto unlock;
+  rc = 0;
+
+unlock:
+  _hash_drv_lock_free(&s, username);
+  return rc;
 }