[bug-gv] [PATCH] fix some corner cases of file_getTmpFile

[Bernhard R. Link]

file_getTmpFile now returns NULL if it cannot create a file
(and all users of this function check for the return).

It always creates the file, so it should be secure even when
mkstemp is not available, avoiding security problems on systems
without it. It also can return the filedescriptor so functions
calling it do not need to repopen.

Set more secure umask for mkstemp, as that is not guaranteed
to use proper file permissions otherwise.
---
 src/file.c    |   36 ++++++++++++++++++++++++------------
 src/file.h    |    5 +++--
 src/options.c |    3 ++-
 src/ps.c      |   34 ++++++++++++++++++++++++----------
 src/save.c    |   20 +++++++++++++-------
 5 files changed, 66 insertions(+), 32 deletions(-)

diff --git a/src/file.c b/src/file.c
index 2b275ec..71549f4 100644
--- a/src/file.c
+++ b/src/file.c
@@ -48,6 +48,7 @@
 #include <time.h>
 #include <ctype.h>
 #include <errno.h>
+#include <fcntl.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -152,9 +153,7 @@ static void file_assureDirectory(char *to, const char *from)
 /*############################################################*/
 
 char *
-file_getTmpFilename(baseDirectory,baseFilename)
-   char *baseDirectory;
-   char *baseFilename;
+file_getTmpFilename(const char *baseDirectory, const char *baseFilename, int 
*filed)
 {
    char tempFilename[256];
    char *tempFilenameP;
@@ -192,21 +191,23 @@ file_getTmpFilename(baseDirectory,baseFilename)
       if (strlen(tmpName)+strlen(tmpExt)>23) tmpName[23-strlen(tmpExt)] = '\0';
    }
    {
-#ifndef HAVE_MKSTEMP
-      struct stat s;
-#endif
-      int no_such_file;
+      int done = 0;
       int i=1;
       do {
+         int fd;
 #ifdef HAVE_MKSTEMP
+        mode_t oldumask;
 #ifdef VMS
          sprintf(tempFilename,"%sgv_%s_%s.XXXXXX",tmpDirBuf,tmpName,tmpExt);
 #else
          sprintf(tempFilename,"%sgv_%s.%s.XXXXXX",tmpDirBuf,tmpName,tmpExt);
 #endif
          file_translateTildeInPath(tempFilename);
-         no_such_file = 1;
-         close(mkstemp(tempFilename));
+        oldumask = umask(0077);
+        fd = mkstemp(tempFilename);
+        umask(oldumask);
+        if (fd < 0)
+                break;
 #else
 #ifdef VMS
          
sprintf(tempFilename,"%sgv_%lx_%x_%s_%s.tmp",tmpDirBuf,time(NULL),i,tmpName,tmpExt);
@@ -214,11 +215,22 @@ file_getTmpFilename(baseDirectory,baseFilename)
          
sprintf(tempFilename,"%sgv_%lx_%x_%s.%s.tmp",tmpDirBuf,time(NULL),i,tmpName,tmpExt);
 #endif
          file_translateTildeInPath(tempFilename);
-         no_such_file = stat(tempFilename,&s);
+        fd = open(tempFilename, O_CREAT|O_EXCL|O_WRONLY, 0600);
 #endif
+        if (fd >= 0) {
+               if (filed)
+                        *filed = fd;
+                else
+                        close(fd);
+               done = 1;
+        }
          i++;
-      } while (!no_such_file);
-   } 
+      } while (!done && i <= 10000);
+      if (!done) {
+          ENDMESSAGE(file_getTmpFilename)
+          return NULL;
+      }
+   }
    SMESSAGE(tempFilename)
    tempFilenameP = GV_XtNewString(tempFilename);
    ENDMESSAGE(file_getTmpFilename)
diff --git a/src/file.h b/src/file.h
index 8df2db3..8729f13 100644
--- a/src/file.h
+++ b/src/file.h
@@ -53,8 +53,9 @@ extern char*                  file_locateFilename (
 
 extern char*                   file_getTmpFilename (
 #if NeedFunctionPrototypes
-   char *,
-   char *
+   const char *,
+   const char *,
+   int *
 #endif
 );
 
diff --git a/src/options.c b/src/options.c
index 2219508..c5b500c 100644
--- a/src/options.c
+++ b/src/options.c
@@ -395,7 +395,8 @@ void options_save(argn,argi,argv)
   INFSMESSAGE(trying to write to,gv_user_defaults_file)
 
   infile=fopen(gv_user_defaults_file,"r"); 
-  
tempfilename=file_getTmpFilename(gv_user_defaults_file,gv_user_defaults_file);
+  tempfilename=file_getTmpFilename(gv_user_defaults_file,
+                 gv_user_defaults_file, NULL);
   INFSMESSAGE(using temporary file,tempfilename)
    
   if (!tempfilename || !(tempfile = fopen(tempfilename,"w"))) {
diff --git a/src/ps.c b/src/ps.c
index 64e788a..8c94d96 100644
--- a/src/ps.c
+++ b/src/ps.c
@@ -410,7 +410,14 @@ 
psscan(fileP,filename,filename_raw,filename_dscP,cmd_scan_pdf,filename_uncP,cmd_
       char cmd[512];
       char s[512];
       mode_t old_umask;
-      filename_unc=file_getTmpFilename(NULL,filename_raw);
+      filename_unc=file_getTmpFilename(NULL, filename_raw, NULL);
+      if (!filename_unc) {
+       NotePopupShowMessage("Cannot create temporary file!");
+       ENDMESSAGE(psscan)
+        return(retval);
+      }
+      old_umask = umask(0077);
+
       quoted_filename = quote_filename(filename);
       quoted_filename_unc = quote_filename(filename_unc);
       if (memcmp(b, "BZh", 3) == 0) {
@@ -421,7 +428,6 @@ 
psscan(fileP,filename,filename_raw,filename_dscP,cmd_scan_pdf,filename_uncP,cmd_
       GV_XtFree(quoted_filename);
       GV_XtFree(quoted_filename_unc);
 
-      old_umask = umask(0077);
 
       INFMESSAGE(is compressed)
       INFSMESSAGE(uncompress command,cmd)
@@ -520,7 +526,14 @@ unc_ok:
       String tmp_filename;
       int tmpfd, tmp_fd;
 
-      filename_dsc=file_getTmpFilename(NULL,filename_raw);
+      filename_dsc=file_getTmpFilename(NULL, filename_raw, NULL);
+      if (!filename_dsc) {
+       NotePopupShowMessage("Cannot create temporary file!");
+       if (tmpfile) fclose(tmpfile);
+        ps_io_exit(fd);
+       ENDMESSAGE(psscan)
+        return(retval);
+      }
       /*      sprintf(cmd,cmd_scan_pdf,filename,filename_dsc); */
       quoted_filename = quote_filename(filename);
       quoted_filename_dsc = quote_filename(filename_dsc);
@@ -560,13 +573,15 @@ unc_ok:
       old_umask = umask(0077);
       INFMESSAGE(is PDF)
       INFSMESSAGE(scan command,cmd)
-      
-      tmp_filename=file_getTmpFilename(NULL,filename_raw);
 
-      tmp_fd = open(tmp_filename, O_WRONLY|O_CREAT|O_TRUNC, 0666);
+      tmp_filename=file_getTmpFilename(NULL, filename_raw, &tmp_fd);
+      if (!tmp_filename) {
+       strcpy(s, "Cannot create temporary file!");
+       goto scan_failed;
+      }
       tmpfd = dup(2);
-      close(2); dup2( tmp_fd, 2);
-      
+      close(2); dup2( tmp_fd, 2); close(tmp_fd);
+
 
       if (system(cmd) || file_fileIsNotUseful(filename_dsc)) {
         char line[1000];
@@ -579,8 +594,7 @@ unc_ok:
        close(2);
        dup2(tmpfd,2);
        close(tmpfd);
-       close(tmp_fd);
-       
+
         tmp_file = fopen( tmp_filename, "r" );
        while ( fgets( line, 999, tmp_file) )
        {
diff --git a/src/save.c b/src/save.c
index 09eaf72..f6551c2 100644
--- a/src/save.c
+++ b/src/save.c
@@ -343,8 +343,10 @@ save_saveFile(sd)
      s = GV_XtNewString(s);
      s = file_getUsefulName(s);
      s = file_pdfname2psname(s);
-     sd->conv_fn = file_getTmpFilename(NULL,s);
+     sd->conv_fn = file_getTmpFilename(NULL, s, NULL);
      GV_XtFree(s);
+     if (sd->conv_fn == NULL)
+            return XtNewString("Cannot create temporary file!");
      INFSMESSAGE(converting from file,sd->src_fn)
      INFSMESSAGE(converting to file,sd->conv_fn)
      error = save_forkPDFToPSConversion(sd);
@@ -359,14 +361,18 @@ save_saveFile(sd)
       if (!sd->save_fn) {
        s = GV_XtNewString(sd->src_fn);
        s = file_getUsefulName(s);
-       s = file_pdfname2psname(s);      
-       sd->save_fn = file_getTmpFilename(NULL,s);
+       s = file_pdfname2psname(s);
+       sd->save_fn = file_getTmpFilename(NULL, s, NULL);
        GV_XtFree(s);
       }
-      INFSMESSAGE(saving from file,src_fn)
-      INFSMESSAGE(saving to file,sd->save_fn)
-      error = save_copyToFile(sd->save_fn,src_fn,sd->pagelist,sd->scanstyle);
-      src_fn = sd->save_fn;
+      if (!sd->save_fn) {
+        error = XtNewString("Cannot create temporary file!");
+      } else {
+        INFSMESSAGE(saving from file,src_fn)
+        INFSMESSAGE(saving to file,sd->save_fn)
+        error = save_copyToFile(sd->save_fn,src_fn,sd->pagelist,sd->scanstyle);
+        src_fn = sd->save_fn;
+      }
    }
 
    if (!error && sd->save_to_printer) {
-- 
1.5.6.5

        Bernhard R. Link
-- 
"Never contain programs so few bugs, as when no debugging tools are available!"
        Niklaus Wirth