LYNX-DEV Improvement on /tmp code fix?

[Jonathan Sergent]

[long, sorry if it's too long]

I finally got around to looking at fotemods/src/LYUtils.c:tempname() 
and it looks like there's still quite a window of opportunity for 
people to exploit a nasty race condition (i.e. they can make the 
symlinks after tempname() does its checks but before the file is 
created, in another process).

I made a diff which uses open(filename, O_CREAT|O_EXCL|O_RDWR, 0600)
which means that tempname() creates the file mode 0600 when it's
called.  The key thing is that it will move on to the next numbered 
file if the open fails, and O_CREAT|O_EXCL means that the open will
fail if the file exists.  If lynx_temp_space is "secure" (i.e. private 
or sticky) then this assures that the file will be okay to use until 
it gets removed.  So I also added checks to see that the temp space 
is either private or sticky, and to give an annoying message (once 
per session) if it isn't.  The files should all get cleaned up
at exit if Lynx works the way I think it does.

Here's a script which illustrates the checks to see if the temp space
is okay (the # lines are my comments)

$ export LYNX_TEMP_SPACE=`pwd`/bad_temp
$ mkdir bad_temp
$ chmod 0777 bad_temp
$ ls -dl bad_temp
drwxrwxrwx  2 sergent  wheel  512 Jul 12 16:47 bad_temp
$ ./lynx
*** ALERT *** Your temp space may be unsafe! Contact your administrator.
# lynx starts normally after a 5-second pause
$ chmod +t bad_temp
$ ls -dl bad_temp
drwxrwxrwt  2 sergent  wheel  512 Jul 12 16:48 bad_temp
$ ./lynx
# lynx starts normally.
$ chmod 700 bad_temp
$ ./lynx
# lynx starts normally
$ chmod 770 bad_temp
$ ./lynx
*** ALERT *** Your temp space may be unsafe! Contact your administrator.
# lynx starts normally after a 5-second pause
$ exit

That code will need an #ifdef for UNIX-only.  I'm not sure what the
portability of fopen -> open will be.  If setting the mode to 0600
at open-time is a problem, doing an fchmod() before closing would
be fine.  A diff to LYUtils.c follows; you also need to add 
UNSAFE_TEMP_DIRECTORY (the above message or something suitably alarming) 
to LYmessages*h.  

BTW, I tested this on BSDI this time.  Sorry if it breaks everything else
in the known universe!


-- 
Jonathan Sergent / address@hidden / address@hidden

file follows:
-rw-r--r--  1 sergent  user  5538 Jul 12 16:52 lyutils-diff

*** LYUtils.c.orig      Sat Jul 12 15:39:41 1997
--- LYUtils.c   Sat Jul 12 16:45:49 1997
***************
*** 3131,3142 ****
        int,            action)
  {
      static int counter = 0;
!     FILE *fp = NULL;
      int LYMaxTempCount = 10000; /* Arbitrary limit.  Make it configurable? */
  
      if (action == REMOVE_FILES) {
          /*
         *  Remove all temporary files with .txt or .html suffixes. - FM
         */ 
        for (; counter > 0; counter--) {
            sprintf(namebuffer,
--- 3131,3180 ----
        int,            action)
  {
      static int counter = 0;
!     int fp = -1;
      int LYMaxTempCount = 10000; /* Arbitrary limit.  Make it configurable? */
+     struct stat st;
+     static int unsafe_warning = 0;
+ 
+     /*
+      *  Check lynx_temp_space to see if it's a safe place to store temp
+      *  files.  See below for comments on what constitutes a safe place.
+      *  --jss.
+      */
+     if (stat(lynx_temp_space, &st) == 0) {
+         if (TRACE)
+           fprintf(stderr, 
+                   "st_mode %i &isvtx %i &(iwgrp|iwoth) %i\n", 
+                   st.st_mode, 
+                   (st.st_mode & S_ISVTX), 
+                   (st.st_mode & (S_IWGRP | S_IWOTH)));
+         fflush(stderr);
+       if   (((st.st_mode & S_ISVTX) != S_ISVTX)
+          && ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0)) {
+                 /* 
+                *  Ack!  We're not safe! --jss. 
+                */
+               if (unsafe_warning == 0) {
+                   fprintf(stderr, "%s\n", UNSAFE_TEMP_DIRECTORY);
+                   sleep(AlertSecs);
+                   unsafe_warning++;
+               }
+         } 
+     } else {
+       /* 
+        *  If we can't stat the temp directory, there's no way it'll work! 
+        *  --jss.
+        */
+       fprintf(stderr, "%s\n", CANNOT_OPEN_TEMP);
+       sleep(AlertSecs);
+       exit(-1);
+     }
+ 
  
      if (action == REMOVE_FILES) {
          /*
         *  Remove all temporary files with .txt or .html suffixes. - FM
+          *  Remove .bin files too, I guess.  --jss.
         */ 
        for (; counter > 0; counter--) {
            sprintf(namebuffer,
***************
*** 3144,3149 ****
--- 3182,3191 ----
                    lynx_temp_space, getpid(), counter-1);
            remove(namebuffer);
            sprintf(namebuffer,
+                   "%sL%d%uTMP.bin",
+                   lynx_temp_space, getpid(), counter-1);
+           remove(namebuffer);
+           sprintf(namebuffer,
                    "%sL%d%uTMP.html",
                    lynx_temp_space, getpid(), counter-1);
            remove(namebuffer);
***************
*** 3162,3202 ****
             *  with the name which has the .html suffix
             *  loaded in namebuffer. - FM
             */
            sprintf(namebuffer,
                    "%sL%d%uTMP.txt",
                    lynx_temp_space, getpid(), counter);
!           if ((fp = fopen(namebuffer, "r")) != NULL) {
!               fclose(fp);
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exits!\n",
                            namebuffer);
                counter++;
                continue;
!           }
            sprintf(namebuffer,
                    "%sL%d%uTMP.bin",
                    lynx_temp_space, getpid(), counter);
!           if ((fp = fopen(namebuffer, "r")) != NULL) {
!               fclose(fp);
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exits!\n",
                            namebuffer);
                counter++;
                continue;
!           }
            sprintf(namebuffer,
                    "%sL%d%uTMP.html",
                    lynx_temp_space, getpid(), counter++);
!           if ((fp = fopen(namebuffer, "r")) != NULL) {
!               fclose(fp);
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exits!\n",
                            namebuffer);
                continue;
!           }
            /*
             *  Return to the calling function, with the tentative
             *  temporary file name loaded in namebuffer.  Note that
--- 3204,3256 ----
             *  with the name which has the .html suffix
             *  loaded in namebuffer. - FM
             */
+             /*
+              *  We actually create these files if we can for stronger anti-
+            *  spoof protection.  We create them mode 0600, so that others 
+            *  can't do nasty things to them, hopefully.  Using open() with 
+            *  O_CREAT and O_EXCL is an atomic operation to check if the 
+            *  file exists and create it if necessary.  The fact that we 
+            *  close them and open them again means that this will fail 
+            *  miserably if others can erase them after we make them, so
+            *  it is only safe to use this code if:
+            *  (a) lynx_temp_space is writeable only by one user, or
+            *  (b) lynx_temp_space is a "sticky directory".
+            */
            sprintf(namebuffer,
                    "%sL%d%uTMP.txt",
                    lynx_temp_space, getpid(), counter);
!           if ((fp = open(namebuffer, O_CREAT|O_EXCL|O_RDWR, 0600)) == -1) {
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exists!\n",
                            namebuffer);
                counter++;
                continue;
!           } else 
!                 close(fp);
            sprintf(namebuffer,
                    "%sL%d%uTMP.bin",
                    lynx_temp_space, getpid(), counter);
!           if ((fp = open(namebuffer, O_CREAT|O_EXCL|O_RDWR, 0600)) == -1) {
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exists!\n",
                            namebuffer);
                counter++;
                continue;
!           } else
!               close(fp);
            sprintf(namebuffer,
                    "%sL%d%uTMP.html",
                    lynx_temp_space, getpid(), counter++);
!           if ((fp = open(namebuffer, O_CREAT|O_EXCL|O_RDWR, 0600)) == -1) {
                if (TRACE)
                    fprintf(stderr,
!                           "tempname: file '%s' already exists!\n",
                            namebuffer);
                continue;
!           } else
!               close(fp);
            /*
             *  Return to the calling function, with the tentative
             *  temporary file name loaded in namebuffer.  Note that
;
; To UNSUBSCRIBE:  Send a mail message to address@hidden
;                  with "unsubscribe lynx-dev" (without the
;                  quotation marks) on a line by itself.
;