Security issue: Repaired tempfile creation race condition.

    Include file special parsing does not access unrelated variables
any more.


git-svn-id: svn://tug.org/texlive/trunk@4531 c570f23f-e606-0410-a88d-b1316a301751

7 files changed, 242 insertions, 113 deletions

diff --git a/Build/source/texk/dviljk/ChangeLog b/Build/source/texk/dviljk/ChangeLog
index 65354398eb6..51419944bb4 100644
--- a/Build/source/texk/dviljk/ChangeLog
+++ b/Build/source/texk/dviljk/ChangeLog
@@ -1,3 +1,14 @@
+2007-07-02  Joachim Schrod  <jschrod@acm.org>
+
+	* dvi2xx.c (DoSpecial): Security issue: usage of tmpnam() caused
+	tempfile race condition. I use mkdtemp() if it's available and
+	fall back to tmpnam.
+
+	Special parsing of include files was inconsistent, unify it. The
+	current parsing code still allows lots of non-sensical special
+	commands, but at least it doesn't access unrelated variables any
+	more.
+
 2007-06-28  Joachim Schrod  <jschrod@acm.org>
 
 	* dvi2xx.c: Fixed a whole bunch of buffer overflows: The program
diff --git a/Build/source/texk/dviljk/c-auto.in b/Build/source/texk/dviljk/c-auto.in
index 7dc1a3403c2..7dadda2ff62 100644
--- a/Build/source/texk/dviljk/c-auto.in
+++ b/Build/source/texk/dviljk/c-auto.in
@@ -1,9 +1,23 @@
-/* c-auto.in.  Generated automatically from configure.in by autoheader.  */
+/* c-auto.in.  Generated from configure.in by autoheader.  */
+/* acconfig.h -- used by autoheader when generating c-auto.in.
 
-/* Define if type char is unsigned and you are not using gcc.  */
-#ifndef __CHAR_UNSIGNED__
-#undef __CHAR_UNSIGNED__
-#endif
+   If you're thinking of editing acconfig.h to fix a configuration
+   problem, don't. Edit the c-auto.h file created by configure,
+   instead.  Even better, fix configure to give the right answer.  */
+
+/* Define to 1 if you have the `mkdtemp' function. */
+#undef HAVE_MKDTEMP
+
+/* Define to 1 if you have the `rmdir' function. */
+#undef HAVE_RMDIR
 
-/* The number of bytes in a long.  */
+/* Define to 1 if you have the `unlink' function. */
+#undef HAVE_UNLINK
+
+/* The size of a `long', as computed by sizeof. */
 #undef SIZEOF_LONG
+
+/* Define to 1 if type `char' is unsigned and you are not using gcc.  */
+#ifndef __CHAR_UNSIGNED__
+# undef __CHAR_UNSIGNED__
+#endif
diff --git a/Build/source/texk/dviljk/config.h b/Build/source/texk/dviljk/config.h
index 9d9ffb613c6..d792a5a11b6 100644
--- a/Build/source/texk/dviljk/config.h
+++ b/Build/source/texk/dviljk/config.h
@@ -440,6 +440,21 @@ typedef  FILE *FILEPTR;
 #endif
 #endif
 
+/* If unlink and rmdir are not there, we don't delete the temporary files. */
+#ifndef HAVE_RMDIR
+#define rmdir(dir)
+#endif
+#ifndef HAVE_UNLINK
+#define unlink(file)
+#endif
+
+/* If mkdtemp() does not exist, we have to use tmpnam(). */
+#ifndef HAVE_MKDTEMP
+#define mkdtemp(dir) (tmpnam(dir) ? \
+		      ( mkdir(dir, 0700) == -1 ? NULL : dir ) :	\
+		      ( errno = EINVAL, NULL ) )
+#endif
+
 #ifndef KPATHSEA
 /* FIXME: Should provide a strdup function. But currently this tree is
    only used in connection with kpathsea anyhow. */
diff --git a/Build/source/texk/dviljk/configure b/Build/source/texk/dviljk/configure
index f5044944032..ff9cd1277ce 100755
--- a/Build/source/texk/dviljk/configure
+++ b/Build/source/texk/dviljk/configure
@@ -1283,6 +1283,62 @@ EOF
 
 
 
+for ac_func in rmdir unlink mkdtemp
+do
+echo $ac_n "checking for $ac_func""... $ac_c" 1>&6
+echo "configure:1290: checking for $ac_func" >&5
+if eval "test \"`echo '$''{'ac_cv_func_$ac_func'+set}'`\" = set"; then
+  echo $ac_n "(cached) $ac_c" 1>&6
+else
+  cat > conftest.$ac_ext <<EOF
+#line 1295 "configure"
+#include "confdefs.h"
+/* System header to define __stub macros and hopefully few prototypes,
+    which can conflict with char $ac_func(); below.  */
+#include <assert.h>
+/* Override any gcc2 internal prototype to avoid an error.  */
+/* We use char because int might match the return type of a gcc2
+    builtin and then its argument prototype would still apply.  */
+char $ac_func();
+
+int main() {
+
+/* The GNU C library defines this for functions which it implements
+    to always fail with ENOSYS.  Some functions are actually named
+    something starting with __ and the normal name is an alias.  */
+#if defined (__stub_$ac_func) || defined (__stub___$ac_func)
+choke me
+#else
+$ac_func();
+#endif
+
+; return 0; }
+EOF
+if { (eval echo configure:1318: \"$ac_link\") 1>&5; (eval $ac_link) 2>&5; } && test -s conftest${ac_exeext}; then
+  rm -rf conftest*
+  eval "ac_cv_func_$ac_func=yes"
+else
+  echo "configure: failed program was:" >&5
+  cat conftest.$ac_ext >&5
+  rm -rf conftest*
+  eval "ac_cv_func_$ac_func=no"
+fi
+rm -f conftest*
+fi
+
+if eval "test \"`echo '$ac_cv_func_'$ac_func`\" = yes"; then
+  echo "$ac_t""yes" 1>&6
+    ac_tr_func=HAVE_`echo $ac_func | tr 'abcdefghijklmnopqrstuvwxyz' 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'`
+  cat >> confdefs.h <<EOF
+#define $ac_tr_func 1
+EOF
+ 
+else
+  echo "$ac_t""no" 1>&6
+fi
+done
+
+
 ac_config_files="$ac_config_files Makefile"
 
 trap '' 1 2 15
diff --git a/Build/source/texk/dviljk/configure.in b/Build/source/texk/dviljk/configure.in
index 4b85965c154..2c5765c37ac 100644
--- a/Build/source/texk/dviljk/configure.in
+++ b/Build/source/texk/dviljk/configure.in
@@ -14,5 +14,7 @@ dnl These tests prevent reliable cross-compilation.  Sigh.
 AC_C_CHAR_UNSIGNED
 AC_CHECK_SIZEOF(long)
 
+AC_CHECK_FUNCS(rmdir unlink mkdtemp)
+
 KPSE_CONFIG_FILES([Makefile])
 AC_OUTPUT
diff --git a/Build/source/texk/dviljk/dvi2xx.c b/Build/source/texk/dviljk/dvi2xx.c
index e739e24efd3..de8d5083d9b 100644
--- a/Build/source/texk/dviljk/dvi2xx.c
+++ b/Build/source/texk/dviljk/dvi2xx.c
@@ -3704,6 +3704,8 @@ bool PFlag;
 #endif
   }
   CloseFiles();
+  if ( tmp_dir[0] != '\0' )
+    rmdir (tmp_dir);			/* ignore errors */
   exit(G_errenc);
 }
 
@@ -3902,7 +3904,8 @@ int  n;
 #endif
 {
   char    xs[STRSIZE], ys[STRSIZE];
-  char    *sf = NULL, *psfile = NULL;
+  char    *include_file = NULL;
+  enum    { VerbFile, HPFile, PSFile } file_type;
   float   x,y;
   long4   x_pos, y_pos;
   KeyWord k;
@@ -3928,37 +3931,30 @@ int  n;
     /* get all keyword-value pairs */
     /* for compatibility, single words are taken as file names */
     if ( k.vt == None && access(k.Key, 0) == 0) {
-      if ( sf && !kpse_tex_hush ("special") ) {
-        Warning("More than one \\special file name given. %s ignored", sf);
-	free (sf);
+      if ( include_file && !kpse_tex_hush ("special") ) {
+        Warning("More than one \\special file name given. %s ignored", include_file);
+	free (include_file);
       }
-      sf = xstrdup(k.Key);
-      /*
-        for (j = 1; ((sf[j]=='/' ? sf[j]='\\':sf[j]) != '\0'); j++);
-        */
+      include_file = xstrdup(k.Key);
+      file_type = VerbFile;
     } else if ( GetKeyVal( &k, KeyTab, NKEYS, &i ) && i != -1 ) {
       switch (i) {
       case PSFILE:
-        if ( sf && !kpse_tex_hush ("special") ) {
-	  Warning("More than one \\special file name given. %s ignored", sf);
-	  free(sf);
-	  sf = NULL;
+        if ( include_file ) {
+	  Warning("More than one \\special file name given. %s ignored", include_file);
+	  free(include_file);
 	}
-        psfile = xstrdup(k.Val);
-        /*
-          for (j=1; ((sf[j]=='/' ? sf[j]='\\':sf[j]) != '\0'); j++);
-          */
+        include_file = xstrdup(k.Val);
+	file_type = PSFile;
         break;
 
       case HPFILE:
-        if ( sf && !kpse_tex_hush ("special") ) {
-	  Warning("More than one \\special file name given. %s ignored", sf);
-	  free(sf);
+        if ( include_file && !kpse_tex_hush ("special") ) {
+	  Warning("More than one \\special file name given. %s ignored", include_file);
+	  free(include_file);
 	}
-        sf = xstrdup(k.Val);
-        /*
-          for (j=1; ((sf[j]=='/' ? sf[j]='\\':sf[j]) != '\0'); j++);
-          */
+        include_file = xstrdup(k.Val);
+	file_type = HPFile;
         break;
 
       case ORIENTATION:
@@ -4097,106 +4093,142 @@ int  n;
     if ( k.Val != NULL )  free(k.Val);
   }
 
-  if ( sf || psfile ) {
+  if ( include_file ) {
     last_rx = last_ry = UNKNOWN;
 #ifdef IBM3812
     PMPflush;
 #endif
-    if (sf) {
-      /* -js FIXME: i may not be HPFILE any more if there were any other
-	 keywords in this special! Need to rework the whole file
-	 inclusion code. */
-      if (i == HPFILE)
-        CopyHPFile( sf );
-      else
-        CopyFile( sf );
-    }
-    else
+
 #ifdef LJ
-      if (psfile) {
-        /* int height = rwi * (urx - llx) / (ury - lly);*/
-        int width  = urx - llx;
-        int height = ury - lly;
-        char cmd[255];
-	char *cmd_format = "%s -q -dSIMPLE -dSAFER -dNOPAUSE -sDEVICE=%s -sOutputFile=%s %s %s showpage.ps -c quit";
-	char *gs_cmd;
-        int scale_factor, adjusted_height, adjusted_llx;
-        char *printer = "ljetplus"; /* use the most stupid one */
-
-
-        char scale_file_name[255];
-        char *scale_file = tmpnam(scale_file_name);
-        char *pcl_file = tmpnam(NULL);
-        FILEPTR scalef;
-
-        if ( urx == 0 || ury == 0 || rwi == 0 ) {
-	  /* Since dvips' psfile special has a different syntax, this might
-	     well be one of those specials, i.e., a non-dviljk special. Then
-	     the Warning should be suppressable. */
-	  if ( !kpse_tex_hush ("special") )
-	    Warning ("Ignoring psfile special without urx, ury and rwi attributes");
+    if ( file_type == PSFile) {
+      /* int height = rwi * (urx - llx) / (ury - lly);*/
+      int width  = urx - llx;
+      int height = ury - lly;
+      char cmd[255];
+      char *cmd_format = "%s -q -dSIMPLE -dSAFER -dNOPAUSE -sDEVICE=%s -sOutputFile=%s %s %s showpage.ps -c quit";
+      char *gs_cmd;
+      int scale_factor, adjusted_height, adjusted_llx;
+      char *printer = "ljetplus"; /* use the most stupid one */
+
+      char pcl_file[STRSIZE];
+      char scale_file[STRSIZE];
+      FILEPTR scalef;
+
+      if ( urx == 0 || ury == 0 || rwi == 0 ) {
+	/* Since dvips' psfile special has a different syntax, this might
+	   well be one of those specials, i.e., a non-dviljk special. Then
+	   the Warning should be suppressable. */
+	if ( !kpse_tex_hush ("special") )
+	  Warning ("Ignoring psfile special without urx, ury and rwi attributes");
+	free (include_file);
+	return;
+      }
+      scale_factor    = 3000 * width / rwi;
+      adjusted_height = height * 300/scale_factor;
+      adjusted_llx    = llx    * 300/scale_factor;
+
+      /* We cannot use mkstemp, as we cannot pass two open file descriptors
+	 portably to Ghostscript. We don't want to use tmpnam() or tempnam()
+	 either, as they have tempfile creation race conditions. Instead we
+	 create a temporary directory with mkdtemp() -- if that's available.
+	 If not, we are thrown back to tempnam(), to get our functionality
+	 at all. We need to create the temporary directory only once per
+	 run; it will be deleted in AllDone(). */
+      if ( tmp_dir[0] == '\0' ) {
+	char * base_dir;
+	if ( (base_dir = getenv("TMPDIR")) == NULL ) {
+	  base_dir = "/tmp";
+	} else if ( strlen(base_dir) > STRSIZE - sizeof("/dviljkXXXXXX/include.pcl") ) {
+	  Warning ("TMPDIR %s is too long, using /tmp instead", base_dir);
+	  base_dir = "/tmp";
+	}
+	if ( base_dir[0] == '/'  && base_dir[1] == '\0' ) {
+	  Warning ("Feeling naughty, do we? / is no temporary directory, dude");
+	  base_dir = "/tmp";
+	}
+	strcpy (tmp_dir, base_dir);
+	strcat (tmp_dir, "/dviljkXXXXXX");
+	if ( mkdtemp(tmp_dir) == NULL ) {
+	  Warning ("Could not create temporary directory %s, errno = %d; ignoring include file special",
+		   tmp_dir, errno);
 	  return;
 	}
-	scale_factor    = 3000 * width / rwi;
-	adjusted_height = height * 300/scale_factor;
-	adjusted_llx    = llx    * 300/scale_factor;
-
-        if ( (scalef = BOUTOPEN(scale_file)) == FPNULL ) {
-          Warning("Unable to open file %s for writing", scale_file );
-          return;
-        }
-        fprintf(scalef, "%.2f %.2f scale\n%d %d translate\n",
-                300.0/scale_factor, 300.0/scale_factor,
-                0, adjusted_height == height ? 0 : ury);
-        BCLOSE( scalef );
+      }
+      strcpy(pcl_file, tmp_dir);
+      strcat(pcl_file, "/include.pcl");
+      strcpy(scale_file, tmp_dir);
+      strcat(scale_file, "/scale.ps");
+
+      if ( (scalef = BOUTOPEN(scale_file)) == FPNULL ) {
+	Warning("Unable to open file %s for writing", scale_file );
+	free (include_file);
+	unlink(scale_file);		/* ignore error */
+	return;
+      }
+      fprintf(scalef, "%.2f %.2f scale\n%d %d translate\n",
+	      300.0/scale_factor, 300.0/scale_factor,
+	      0, adjusted_height == height ? 0 : ury);
+      BCLOSE( scalef );
 
 #ifdef WIN32
-	gs_cmd = ( getenv("GS_PATH") || "gswin32c.exe" );
+      if ( (gs_cmd = getenv("GS_PATH")) == NULL )
+	gs_cmd = "gswin32c.exe";
 #else
-	gs_cmd = "gs";
-#endif
-	if ( strlen(cmd_format)-10 + strlen(gs_cmd) + strlen(printer) +
-	         strlen(pcl_file) + strlen(scale_file) + strlen(psfile) +1 >
-	     sizeof(cmd) ) {
-	  Warning ("Ghostscript command for %s would be too long, skipping special", psfile);
-	  return;
-	}
-        sprintf(cmd, cmd_format,
-		gs_cmd, printer, pcl_file, scale_file, psfile);
+      gs_cmd = "gs";
+#endif
+      if ( strlen(cmd_format)-10 + strlen(gs_cmd) + strlen(printer) +
+	       strlen(pcl_file) + strlen(scale_file) + strlen(include_file) +1 >
+	   sizeof(cmd) ) {
+	Warning ("Ghostscript command for %s would be too long, skipping special", include_file);
+	free (include_file);
+	unlink(scale_file);		/* ignore errors */
+	unlink(pcl_file);
+	return;
+      }
+      sprintf(cmd, cmd_format,
+	      gs_cmd, printer, pcl_file, scale_file, include_file);
 #ifdef DEBUGGS
-        fprintf(stderr,
-          "PS-file '%s' w=%d, h=%d, urx=%d, ury=%d, llx=%d, lly=%d, rwi=%d\n",
-                psfile, urx - llx, height, urx,ury,llx,lly, rwi);
-        fprintf(stderr,"%s\n",cmd);
+      fprintf(stderr,
+	"PS-file '%s' w=%d, h=%d, urx=%d, ury=%d, llx=%d, lly=%d, rwi=%d\n",
+	      include_file, urx - llx, height, urx,ury,llx,lly, rwi);
+      fprintf(stderr,"%s\n",cmd);
 #endif
-        if (system(cmd)) {
-          Warning("execution of '%s' returned an error", cmd);
-        } else {
+      if (system(cmd)) {
+	Warning("execution of '%s' returned an error", cmd);
+      } else {
 #ifdef DEBUGGS
-          fprintf(stderr, "o=%d, h=%d, so=%d, sh=%d\n",
-                  llx, height, adjusted_llx, adjusted_height);
+	fprintf(stderr, "o=%d, h=%d, so=%d, sh=%d\n",
+		llx, height, adjusted_llx, adjusted_height);
 
-          fprintf(stderr, "OLD x=%d, y=%d\n",
-                  (int)PIXROUND(h, hconv) + x_goffset,
-                  (int)PIXROUND(v, vconv) + y_goffset);
+	fprintf(stderr, "OLD x=%d, y=%d\n",
+		(int)PIXROUND(h, hconv) + x_goffset,
+		(int)PIXROUND(v, vconv) + y_goffset);
 #endif
-          v -= 65536l*adjusted_height; /**300/scale_factor;*/
-          h -= 65536l*adjusted_llx; /* *300/scale_factor;*/
-          SetPosn(h, v);
+	v -= 65536l*adjusted_height; /**300/scale_factor;*/
+	h -= 65536l*adjusted_llx; /* *300/scale_factor;*/
+	SetPosn(h, v);
 #ifdef DEBUGGS
-          fprintf(stderr, "NEW x=%d, y=%d\n",
-                  (int)PIXROUND(h, hconv) + x_goffset,
-                  (int)PIXROUND(v, vconv) + y_goffset);
+	fprintf(stderr, "NEW x=%d, y=%d\n",
+		(int)PIXROUND(h, hconv) + x_goffset,
+		(int)PIXROUND(v, vconv) + y_goffset);
 #endif
 
-          CopyHPFile( pcl_file );
-          /* unlink(pcl_file); */
-          /* unlink(scale_file); */
-        }
+	CopyHPFile( pcl_file );
       }
+      unlink(scale_file);		/* ignore errors */
+      unlink(pcl_file);
+    }
+    else
 #endif /* LJ */
-    if ( sf != NULL )  free(sf);
-    if ( psfile != NULL )  free(psfile);
+
+    if ( file_type == HPFile )
+      CopyHPFile( include_file );
+    else if ( file_type == VerbFile )
+      CopyFile( include_file );
+    else
+      Warning ("This can't happen: unknown file_type value %d", file_type);
+
+    if ( include_file != NULL )  free(include_file);
   }
 }
 
diff --git a/Build/source/texk/dviljk/dvi2xx.h b/Build/source/texk/dviljk/dvi2xx.h
index 15d37a59efa..333c493bc7b 100644
--- a/Build/source/texk/dviljk/dvi2xx.h
+++ b/Build/source/texk/dviljk/dvi2xx.h
@@ -10,8 +10,8 @@
 
 #ifdef KPATHSEA
 #include <kpathsea/config.h>
+#include <kpathsea/c-std.h>
 #include <kpathsea/c-limits.h>
-#include <kpathsea/c-memstr.h>
 #include <kpathsea/magstep.h>
 #include <kpathsea/proginit.h>
 #include <kpathsea/progname.h>
@@ -24,6 +24,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <unistd.h>
 #ifdef  unix
 #include <limits.h>
 #endif
@@ -41,9 +42,6 @@
 #ifdef MSC5
 #include <dos.h>     /* only for binaryopen on device  */
 #endif
-#if defined (unix) && !defined (KPATHSEA)
-#include <limits.h>
-#endif
 
 
 #include "config.h"
@@ -548,6 +546,7 @@ char   *filename;	         /* DVI file name                       */
 char   *rootname;		 /* DVI filename without extension      */
 char   *HeaderFileName = "";     /* file name & path of Headerfile      */
 char   *EmitFileName = "";       /* file name & path for output         */
+char    tmp_dir[STRSIZE] = "";	 /* temporary directory for auxilliary files */
 #ifdef IBM3812
 bool    FirstAlternate = _FALSE; /* first page from alternate casette ?   */
 #endif