Bug 16929

>
>sort can create the following predictable tempfiles:
>/tmp/sort{pid}{seq}

It appears that the security implications of this have already been
fixed in rev.1.11 of src/gnu/usr.bin/sort/sort.c.


>   Fix
>          
>Since sort can create many tempfiles, we should leave it's current
>naming scheme alone, rather create a secure dir in TMP with mkdtemp(3),
>and let sort dumps it's file in there.
>
>Apply the following patch, sorry there might be whitespace bugs =(
>
>Index: gnu/usr.bin/sort/sort.c
>===================================================================
>RCS file: /home/ncvs/src/gnu/usr.bin/sort/sort.c,v
>retrieving revision 1.15
>diff -u -r1.15 sort.c
>--- sort.c      1999/04/25 22:14:05     1.15
>+++ sort.c      2000/02/23 06:45:13
>@@ -171,6 +171,8 @@
>
> /* Prefix for temporary file names. */
> static char *temp_file_prefix;
>+/* Temporary dir for temp files, *with* above prefix */
>+static char *temp_dir = NULL;
>
> /* Flag to reverse the order of all comparisons. */
> static int reverse;
>@@ -288,6 +290,9 @@
>
>   for (node = temphead.next; node; node = node->next)
>     unlink (node->name);
>+  if( temp_dir )
>+    rmdir(temp_dir);
>+
> }
>
> /* Allocate N bytes of memory dynamically, with error checking.  */
>@@ -413,6 +418,7 @@
>     }
> }
>
>+#define DIR_TEMPLATE    "sortXXXXXXXXXX"
> /* Return a name for a temporary file. */
>
> static char *
>@@ -420,15 +426,29 @@
> {
>   static unsigned int seq;
>   int len = strlen (temp_file_prefix);
>-  char *name = xmalloc (len + 1 + sizeof ("sort") - 1 + 5 + 5 + 1);
>+  char *name=xmalloc(len + 1 + sizeof(DIR_TEMPLATE)-1 + 1 + sizeof("sort")-1 +
> 5 + 5 + 1);
>   struct tempnode *node;
>
>   node = (struct tempnode *) xmalloc (sizeof (struct tempnode));
>+  if( !temp_dir )
>+         {
>+                 temp_dir = xmalloc( len + 1 + sizeof(DIR_TEMPLATE) );
>+                 sprintf(temp_dir,
>+                                 "%s%s%s",
>+                                 temp_file_prefix,
>+                                 (len && temp_file_prefix[len - 1] != '/') ? "
>/" : "",
>+                                 DIR_TEMPLATE);
>+                 if( mkdtemp(temp_dir) == NULL )
>+                         {
>+                                 error(0, errno, _("can't make temp dir"));
>+                                 exit(2);
>+                         }
>+         }
>+
>   sprintf (name,
>-          "%s%ssort%5.5d%5.5d",
>-          temp_file_prefix,
>-          (len && temp_file_prefix[len - 1] != '/') ? "/" : "",
>-          (unsigned int) getpid () & 0xffff, seq);
>+                  "%s/sort%5.5d%5.5d",
>+                  temp_dir,
>+                  (unsigned int) getpid () & 0xffff, seq);
>
>   /* Make sure that SEQ's value fits in 5 digits.  */
>   ++seq;
>
>
>   [4]Submit Followup
>     _________________________________________________________________
>   
>   
>    [5]www@FreeBSD.org

On 16-May-2000 Tim Vanderhoek wrote:
| >
| >sort can create the following predictable tempfiles:
| >/tmp/sort{pid}{seq}
|  
|  It appears that the security implications of this have already been
|  fixed in rev.1.11 of src/gnu/usr.bin/sort/sort.c.
|  

yes, i suppose they have been. however, as sort can create multiple tempfiles it
was suggested that they be kept in one directory per sort process running (see
thread in -audit list), rather than dumping them all in the temp dir.

later,



-
  Mike Heffner  <spock@techfour.net>
  Fredericksburg, VA     ICQ# 882073
  http://my.ispchannel.com/~mheffner
-

On 23-Feb-2000 spock@techfour.net wrote:
| 
|>Number:         16929
|>Category:       bin
|>Synopsis:       [PATCH] prevent possible race condition in sort

This was fixed in rev 1.18, so it can be closed.

-- 

  Mike Heffner       <mheffner@vt.edu>
  Blacksburg, VA           ICQ# 882073
  http://filebox.vt.edu/users/mheffner

Comment 4 Kris Kennaway 2001-02-02 10:46:56 UTC

State Changed
From-To: open->closed

Problem resolved.