[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[dsiebert@ICAEN.UIOWA.EDU: Safe /tmp cleanup]



Maybe we can use this? (Message taken from the bugtraq mailinglist).

Wichert.

-----Forwarded message from dsiebert@ICAEN.UIOWA.EDU-----

Date:         Tue, 11 Nov 1997 11:52:06 -0600
Reply-To: douglas-siebert@uiowa.edu
Sender: Bugtraq List <BUGTRAQ@NETSPACE.ORG>
From: dsiebert@ICAEN.UIOWA.EDU
Organization: Iowa Computer Aided Engineering Network, University of Iowa
Subject:      Safe /tmp cleanup
To: BUGTRAQ@NETSPACE.ORG

There was a thread in Bugtraq a couple months ago about safe cleanup of
/tmp and other publicly-writable directories.  The problem is with the
traditional cleanup along the lines of:

find /tmp -someoptions -print | xargs rm -moreoptions

An attacker can create conditions using deep directories and symbolic
links that will cause this command to delete any arbitrary file on the
filesystem.  See the archives for more info.

This started a long discussion, and only two good solutions were proposed
to my recollection.  One, someone had a Perl script named "saferm" which
did an insane amount of sanity checking to verify the path was correct.
Two, it was proposed that the find command itself should handle this.
The Perl script is quite slow and overly complex, I wanted a better
solution.  I took a look at the GNU archive to see if they had a find
command which might already have such an option.  They had a find command
which hasn't been updated for about three years, which had no such option.
But the source is very easy to read and modify so it was a simple matter
to add a "-delete" option myself.  I also noticed and fixed a bug that
caused incorrect results when using the "-depth" option in some cases
(those of you with Linux boxes, which use the GNU find, can try: "find /var
-depth -empty" and you'll see what I mean)  This was important to do since
you need the -depth option to work for -delete to really work (-delete
implies -depth in my code)

I also went to some trouble to insure that there were no race conditions
when changing directories, by using fchdir() everywhere I could and sanity
checking that the directory you went into was what you thought it was where
I couldn't use fchdir().  Its been said before, but open() _really_ needs
an option to tell it to not follow symbolic links (and if it had been done
right at the start, nothing would default to following symbolic links,
you'd have to specifically indicate this if you wanted it!)

Here are the diffs against GNU find 4.1.  I did not update the man pages or
documentation for this change, if someone wants to submit this change to the
FSF for a GNU find 4.1.1 that will need to be done.  I have not extensively
tested this yet, but it seems to work fine.  If anyone notices any problems
(in the stuff I've coded, not the rest of it!) please let me know.  I'm
posting it here hoping anything I've overlooked will be noticed by the
Bugtraq readership, so we can put this problem behind us for once and for
all.




*** defs.h.orig Wed Nov  2 14:59:15 1994
--- defs.h      Thu Nov  6 09:24:59 1997
***************
*** 249,254 ****
--- 249,255 ----
  boolean pred_cnewer P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
  boolean pred_comma P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
  boolean pred_ctime P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
+ boolean pred_delete P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
  boolean pred_empty P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
  boolean pred_exec P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
  boolean pred_false P_((char *pathname, struct stat *stat_buf, struct predicate *pred_ptr));
*** find.c.orig Wed Oct 12 16:21:11 1994
--- find.c      Thu Nov  6 10:15:44 1997
***************
*** 105,111 ****
  boolean have_stat;

  /* The file being operated on, relative to the current directory.
!    Used for stat, readlink, and opendir.  */
  char *rel_pathname;

  /* Length of current path. */
--- 105,111 ----
  boolean have_stat;

  /* The file being operated on, relative to the current directory.
!    Used for stat, readlink, remove, and opendir.  */
  char *rel_pathname;

  /* Length of current path. */
***************
*** 332,337 ****
--- 332,339 ----
    struct stat stat_buf;
    static dev_t root_dev;      /* Device ID of current argument pathname. */
    int i;
+   struct stat dir_buf;
+   int parent_desc;

    /* Assume it is a non-directory initially. */
    stat_buf.st_mode = 0;
***************
*** 390,398 ****
      apply_predicate (pathname, &stat_buf, eval_tree);

    if (stop_at_current_level == false)
!     /* Scan directory on disk. */
!     process_dir (pathname, name, strlen (pathname), &stat_buf, parent);

    if (do_dir_first == false && curdepth >= mindepth)
      apply_predicate (pathname, &stat_buf, eval_tree);

--- 392,440 ----
      apply_predicate (pathname, &stat_buf, eval_tree);

    if (stop_at_current_level == false)
!     {
!       parent_desc = open (".", O_RDONLY);
!       if (parent_desc < 0)
!         error (1, errno, "%s", pathname);

+       if (chdir (name) < 0)
+       {
+         error (0, errno, "%s", pathname);
+         exit_status = 1;
+         close(parent_desc);
+         return;
+       }
+
+       if ((*xstat) (".", &dir_buf) < 0)
+       error (1, errno, "%s", pathname);
+
+       if (!S_ISDIR (dir_buf.st_mode) || dir_buf.st_ino != stat_buf.st_ino ||
+         dir_buf.st_dev != stat_buf.st_dev)
+       error (1, 0, "Link changed: %s", pathname);
+
+       /* Scan directory on disk. */
+       process_dir (pathname, ".", strlen (pathname), &stat_buf, pathname);
+
+ #ifndef HAVE_FCHDIR
+       if (!dereference)
+       {
+         if (chdir ("..") < 0)
+           error (1, errno, "%s", parent);
+       }
+       else
+       {
+         if (chdir (starting_dir) || chdir (parent))
+           error (1, errno, "%s", parent);
+       }
+ #else
+       if (fchdir (parent_desc) < 0)
+       error (1, errno, "%s", parent);
+ #endif
+       close (parent_desc);
+     }
+
+   rel_pathname = name;
+
    if (do_dir_first == false && curdepth >= mindepth)
      apply_predicate (pathname, &stat_buf, eval_tree);

***************
*** 454,466 ****
        cur_path_size = 0;
        cur_path = NULL;

-       if (strcmp (name, ".") && chdir (name) < 0)
-       {
-         error (0, errno, "%s", pathname);
-         exit_status = 1;
-         return;
-       }
-
        for (namep = name_space; *namep; namep += file_len - pathname_len + 1)
        {
          /* Append this directory entry's name to the path being searched. */
--- 496,501 ----
***************
*** 495,521 ****
               mounted, which don't have Unix-like directory link counts. */
            process_path (cur_path, cur_name, false, pathname);
          curdepth--;
-       }
-
-       if (strcmp (name, "."))
-       {
-         if (!dereference)
-           {
-             if (chdir ("..") < 0)
-               /* We could go back and do the next command-line arg instead,
-                  maybe using longjmp.  */
-               error (1, errno, "%s", parent);
-           }
-         else
-           {
- #ifndef HAVE_FCHDIR
-             if (chdir (starting_dir) || chdir (parent))
-               error (1, errno, "%s", parent);
- #else
-             if (fchdir (starting_desc) || chdir (parent))
-               error (1, errno, "%s", parent);
- #endif
-           }
        }

        if (cur_path)
--- 530,535 ----
*** parser.c.orig       Wed Nov  2 14:59:19 1994
--- parser.c    Thu Nov  6 09:30:36 1997
***************
*** 78,83 ****
--- 78,84 ----
  static boolean parse_comma P_((char *argv[], int *arg_ptr));
  static boolean parse_ctime P_((char *argv[], int *arg_ptr));
  static boolean parse_daystart P_((char *argv[], int *arg_ptr));
+ static boolean parse_delete P_((char *argv[], int *arg_ptr));
  static boolean parse_depth P_((char *argv[], int *arg_ptr));
  static boolean parse_empty P_((char *argv[], int *arg_ptr));
  static boolean parse_exec P_((char *argv[], int *arg_ptr));
***************
*** 176,181 ****
--- 177,183 ----
  #endif
    {"ctime", parse_ctime},
    {"daystart", parse_daystart},       /* GNU */
+   {"delete", parse_delete},
    {"depth", parse_depth},
    {"empty", parse_empty},     /* GNU */
    {"exec", parse_exec},
***************
*** 427,432 ****
--- 429,448 ----
  }

  static boolean
+ parse_delete (argv, arg_ptr)
+      char *argv[];
+      int *arg_ptr;
+ {
+   struct predicate *our_pred;
+
+   our_pred = insert_primary (pred_delete);
+   our_pred->side_effects = true;
+   /* -delete implies -depth */
+   do_dir_first = false;
+   return (true);
+ }
+
+ static boolean
  parse_depth (argv, arg_ptr)
       char *argv[];
       int *arg_ptr;
***************
*** 611,617 ****
        ( EXPR ) ! EXPR -not EXPR EXPR1 -a EXPR2 EXPR1 -and EXPR2\n");
    printf ("\
        EXPR1 -o EXPR2 EXPR1 -or EXPR2 EXPR1 , EXPR2\n\
! options (always true): -daystart -depth -follow --help\n\
        -maxdepth LEVELS -mindepth LEVELS -mount -noleaf --version -xdev\n\
  tests (N can be +N or -N or N): -amin N -anewer FILE -atime N -cmin N\n");
    printf ("\
--- 627,633 ----
        ( EXPR ) ! EXPR -not EXPR EXPR1 -a EXPR2 EXPR1 -and EXPR2\n");
    printf ("\
        EXPR1 -o EXPR2 EXPR1 -or EXPR2 EXPR1 , EXPR2\n\
! options (always true): -daystart -delete -depth -follow --help\n\
        -maxdepth LEVELS -mindepth LEVELS -mount -noleaf --version -xdev\n\
  tests (N can be +N or -N or N): -amin N -anewer FILE -atime N -cmin N\n");
    printf ("\
*** pred.c.orig Wed Nov  2 14:59:23 1994
--- pred.c      Thu Nov  6 09:28:24 1997
***************
*** 108,113 ****
--- 108,114 ----
    {pred_cnewer, "cnewer  "},
    {pred_comma, ",       "},
    {pred_ctime, "ctime   "},
+   {pred_delete, "delete  "},
    {pred_empty, "empty   "},
    {pred_exec, "exec    "},
    {pred_false, "false   "},
***************
*** 376,381 ****
--- 377,393 ----
        break;
      }
    return (false);
+ }
+
+ boolean
+ pred_delete (pathname, stat_buf, pred_ptr)
+      char *pathname;
+      struct stat *stat_buf;
+      struct predicate *pred_ptr;
+ {
+   if (strcmp (rel_pathname, "."))
+     return (remove (rel_pathname));
+   return (true);
  }

  boolean




--
Douglas Siebert                Director of Computing Facilities
douglas-siebert@uiowa.edu      Division of Mathematical Sciences, U of Iowa

If you let the system beat you long enough, eventually it'll get tired.

-----End of forwarded message-----

Attachment: pgpRXWWKWxRuh.pgp
Description: PGP signature


Reply to: