[Groff] Re: groff 1.17: pre-grohtml's unsafe temporary file handling & other improvements

[Gaius Mulley]

Hi Werner,

Werner writes:
> Fixed in the current snapshot -- as soon as Gaius has fixed the
> security bugs in grohtml I'll release 1.17.1.

ahh good timing :-)

here are some patches which include:

  * improved security w.r.t grohtml, thanks from the people at openwall.
  * altering the syntax of \On to \O[2] and \O[5 filename] as Werner
    suggested.
  * new option to grohtml -D, invoked via groff -P-Ddirpng which places all
    images in directory dirpng.
  * another grohtml option -I, groff -P-Ifoobar which defines the image stem:
    such as: foobar-%d.png

Gaius


--- groff-cvs/src/devices/grohtml/grohtml.man   Fri Apr 13 10:03:53 2001
+++ groff-html/src/devices/grohtml/grohtml.man  Mon Jun  4 23:40:07 2001
@@ -37,8 +37,10 @@
 .el .RB "[\ " "\\$1" "\ ]"
 ..
 .OP \-v?lrn
+.OP \-D dir
 .OP \-F dir
 .OP \-i resolution
+.OP \-I image stem
 .OP \-o image vertical offset
 .RI "[\ " files\|.\|.\|. "\ ]"
 .br
@@ -104,6 +106,19 @@
 select the resolution for all images.
 By default this is 80 pixels per inch.
 Example: -i100 indicates 100 pixels per inch.
+.TP
+.BI \-I stem
+determine the image stem name. If omitted grohtml uses
+.BR grohtml-<pid> .
+.TP
+.BI \-D dir
+informs
+.B grohtml
+to place all image files into directory,
+.BR dir .
+.I dir
+for all images.
+.BR grohtml-<pid> .
 .TP
 .B \-v
 Print the version number.
--- groff-cvs/src/devices/grohtml/post-html.cc  Mon Jun  4 11:17:31 2001
+++ groff-html/src/devices/grohtml/post-html.cc Mon Jun  4 18:19:39 2001
@@ -2879,7 +2879,7 @@
     { "version", no_argument, 0, 'v' },
     { NULL, 0, 0, 0 }
   };
-  while ((c = getopt_long(argc, argv, "o:i:F:vd?lrn", long_options, NULL))
+  while ((c = getopt_long(argc, argv, "o:i:I:D:F:vd?lrn", long_options, NULL))
         != EOF)
     switch(c) {
     case 'v':
@@ -2903,6 +2903,12 @@
     case 'i':
       /* handled by pre-html */
       break;
+    case 'I':
+      /* handled by pre-html */
+      break;
+    case 'D':
+      /* handled by pre-html */
+      break;
     case 'n':
       simple_anchors = TRUE;
       break;
@@ -2929,6 +2935,6 @@
 
 static void usage(FILE *stream)
 {
-  fprintf(stream, "usage: %s [-vld?n] [-F dir] [files ...]\n",
+  fprintf(stream, "usage: %s [-vld?n] [-I image_stem] [-F dir] [files ...]\n",
          program_name);
 }
--- groff-cvs/src/preproc/html/pre-html.cc      Fri Apr 20 14:35:01 2001
+++ groff-html/src/preproc/html/pre-html.cc     Mon Jun  4 18:28:51 2001
@@ -58,6 +58,7 @@
 #define IMAGE_BOARDER_PIXELS       0
 #define MAX_WIDTH                  8   // inches
 #define INLINE_LEADER_CHAR      '\\'
+#define MAX_RETRIES             4096   // number of different page directory 
names to try before giving up
 
 #define TRANSPARENT  "-background \"#FFF\" -transparent \"#FFF\""
 
@@ -91,6 +92,7 @@
 static int   troff_arg      = 0;                // troff arg index
 static char *command_prefix = 0;                // optional prefix for some 
installations.
 static char *troff_command  = 0;
+static char *image_dir      = 0;                // user specified image 
directory
 #if defined(DEBUGGING)
 static int   debug          = FALSE;
 static char *troffFileName  = 0;                // output of pre-html output 
which is sent to troff -Tps
@@ -243,17 +245,40 @@
  *  makeFileName - creates the image filename template.
  */
 
-void makeFileName ()
+static void makeFileName (void)
 {
   char buffer[8192];
 
-  sprintf(buffer, "grohtml-%d", (int)getpid());
+  if (image_dir == NULL) {
+    image_dir = (char *)malloc(1);
+    image_dir[0] = (char)0;
+  } else if ((strlen(image_dir)>1) && (image_dir[strlen(image_dir)-1] != '/')) 
{
+    sprintf(buffer, "%s/", image_dir);
+    image_dir = strdup(buffer);
+  }
+
+  if (image_template == NULL)
+    sprintf(buffer, "%sgrohtml-%d", image_dir, (int)getpid());
+  else
+    sprintf(buffer, "%s%s", image_dir, image_template);
+
   strcat(buffer, "-%d");
   image_template = (char *)malloc(strlen(buffer)+1);
   strcpy(image_template, buffer);
 }
 
 /*
+ *  checkImageDir - checks to see whether the image directory is available.
+ */
+
+static void checkImageDir (void)
+{
+  if ((image_dir != NULL) && (strcmp(image_dir, "") != 0))
+    if (! ((mkdir(image_dir, 0700) == 0) || (errno == EEXIST)))
+      error("cannot create directory `%1'", image_dir);
+}
+
+/*
  *  write_end_image - ends the image. It writes out the image extents if we 
are using -Tps.
  */
 
@@ -263,12 +288,12 @@
     /*
      *  emit image name and enable output
      */
-    writeString("\\O2\\O1\\O4\n");
+    writeString("\\O[2]\\O[1]\\O[4]\n");
   } else {
     /*
      *  postscript, therefore emit image boundaries
      */
-    writeString("\\O2\\O4\n");
+    writeString("\\O[2]\\O[4]\n");
   }
 }
 
@@ -283,8 +308,8 @@
 static void write_start_image (IMAGE_ALIGNMENT pos, int is_html)
 {
   if (pos == INLINE) {
-    writeString("\\O3\\O5'");
-    writeString(image_template); writeString(".png'");
+    writeString("\\O[3]\\O[5 ");
+    writeString(image_template); writeString(".png]");
   } else {
     writeString(".begin \\{\\\n");
     switch (pos) {
@@ -307,10 +332,10 @@
     writeString("\\}\n");
   }
   if (is_html) {
-    writeString("\\O0\n");
+    writeString("\\O[0]\n");
   } else {
     // reset min/max registers
-    writeString("\\O0\\O1\n");
+    writeString("\\O[0]\\O[1]\n");
   }
 }
 
@@ -574,12 +599,38 @@
  *  createAllPages - creates a set of images, one per page.
  */
 
-static void createAllPages (void)
+static int createAllPages (void)
 {
   char buffer[4096];
+  int retries = MAX_RETRIES;
+
+  imagePageStem = xtmptemplate("-page-");
+  strcpy(buffer, imagePageStem);
+
+  do {
+    if (mktemp(imagePageStem) == NULL) {
+      sys_fatal("mktemp");
+      return -1;
+    }
+    if (mkdir(imagePageStem, 0700) == 0) break;
+    if (errno == EEXIST) {
+      // directory already exists, try another name
+      retries--;
+      if (retries == 0) {
+       // time to give up
+       sys_fatal("mkdir");
+       return -1;
+      }
+    } else {
+      // another error, quit
+      sys_fatal("mkdir");
+      return -1;
+    }      
+    strcpy(imagePageStem, buffer);
+  } while (1);
 
   sprintf(buffer,
-         "echo showpage | gs -q -dSAFER -sDEVICE=%s -r%d -sOutputFile=%s%%d %s 
- > /dev/null 2>&1 \n",
+         "echo showpage | gs -q -dSAFER -sDEVICE=%s -r%d -sOutputFile=%s/%%d 
%s - > /dev/null 2>&1 \n",
          image_device,
          image_res,
          imagePageStem,
@@ -589,6 +640,7 @@
   fflush(stderr);
 #endif
   system(buffer);
+  return 0;
 }
 
 /*
@@ -602,9 +654,10 @@
   int  i=1;
 
   do {
-    sprintf(buffer, "%s%d", imagePageStem, i);
+    sprintf(buffer, "%s/%d", imagePageStem, i);
     i++;
-  } while (remove(buffer) == 0);
+  } while (unlink(buffer) == 0);
+  rmdir(imagePageStem);
 #endif
 }
 
@@ -648,7 +701,7 @@
     int  y2 = (image_res*vertical_offset/72)+max(i->Y1, 
i->Y2)*image_res/POSTSCRIPTRES+1*IMAGE_BOARDER_PIXELS;
 
     sprintf(buffer,
-           "pnmcut %d %d %d %d < %s%d | pnmtopng %s > %s \n",
+           "pnmcut %d %d %d %d < %s/%d | pnmtopng %s > %s \n",
            x1, y1, x2-x1+1, y2-y1+1,
            imagePageStem,
            i->pageNo,
@@ -987,13 +1040,15 @@
 
 void usage(FILE *stream)
 {
-  fprintf(stream, "usage: %s troffname [-P-o vertical_image_offset] [-P-i 
image_resolution] [troff flags] [files]\n", program_name);
+  fprintf(stream, "usage: %s troffname [-Iimage_name] [-Dimage_directory] 
[-P-o vertical_image_offset] [-P-i image_resolution] [troff flags] [files]\n", 
program_name);
   fprintf(stream, "    vertical_image_offset (default %d/72 of an inch)\n", 
vertical_offset);
   fprintf(stream, "    image_resolution (default %d) pixels per inch\n", 
image_res);
+  fprintf(stream, "    image_name is the name of the stem for all images 
(default is grohtml-<pid>)\n");
+  fprintf(stream, "    place all png files into image_directory\n");
 }
 
 /*
- *  scanArguments - scans for -P-i and -P-o arguments.
+ *  scanArguments - scans for -P-i, -P-o, -P-D and -P-I arguments.
  */
 
 int scanArguments (int argc, char **argv)
@@ -1001,7 +1056,11 @@
   int i=1;
 
   while (i<argc) {
-    if (strncmp(argv[i], "-i", 2) == 0) {
+    if (strncmp(argv[i], "-D", 2) == 0) {
+      image_dir = (char *)(argv[i]+2);
+    } else if (strncmp(argv[i], "-I", 2) == 0) {
+      image_template = (char *)(argv[i]+2);
+    } else if (strncmp(argv[i], "-i", 2) == 0) {
       image_res = atoi((char *)(argv[i]+2));
     } else if (strncmp(argv[i], "-o", 2) == 0) {
       vertical_offset = atoi((char *)(argv[i]+2));
@@ -1033,7 +1092,7 @@
  *  makeTempFiles - name the temporary files
  */
 
-static void makeTempFiles (void)
+static int makeTempFiles (void)
 {
 #if defined(DEBUGGING)
   psFileName     = "/tmp/prehtml-ps";
@@ -1042,10 +1101,21 @@
   troffFileName  = "/tmp/prehtml-troff";
   htmlFileName   = "/tmp/prehtml-html";
 #else
-  psFileName     = mktemp(xtmptemplate("-ps-"));
-  regionFileName = mktemp(xtmptemplate("-regions-"));
-  imagePageStem  = mktemp(xtmptemplate("-page-"));
+  int fd;
+
+  if ((fd = mkstemp(psFileName = xtmptemplate("-ps-"))) == -1) {
+    sys_fatal("mkstemp");
+    return -1;
+  }
+  close(fd);
+  if ((fd = mkstemp(regionFileName = xtmptemplate("-regions-"))) == -1) {
+    sys_fatal("mkstemp");
+    unlink(psFileName);
+    return -1;
+  }
+  close(fd);
 #endif
+  return 0;
 }
 
 /*
@@ -1055,8 +1125,8 @@
 static void removeTempFiles (void)
 {
 #if !defined(DEBUGGING)
-  remove(psFileName);
-  remove(regionFileName);
+  unlink(psFileName);
+  unlink(regionFileName);
 #endif
 }
 
@@ -1084,8 +1154,9 @@
   int ok=1;
 
   findPrefix();
-  makeFileName();
   i = scanArguments(argc, argv);
+  checkImageDir();
+  makeFileName();
   while (i < argc) {
     if (argv[i][0] != '-') {
       /* found source file */
@@ -1103,13 +1174,16 @@
   if (! found) {
     do_file("-");
   }
-  makeTempFiles();
+  if (makeTempFiles())
+    return 1;
   ok = inputFile.do_image(argc, argv);
   if (ok == 0) {
-    createAllPages();
-    generateImages(regionFileName);
-    ok = inputFile.do_html(argc, argv);
-    removeAllPages();
+    ok = createAllPages();
+    if (ok == 0) {
+      generateImages(regionFileName);
+      ok = inputFile.do_html(argc, argv);
+      removeAllPages();
+    }
   }
   removeTempFiles();
   return ok;
--- groff-cvs/src/roff/troff/input.cc   Mon Jun  4 11:17:50 2001
+++ groff-html/src/roff/troff/input.cc  Mon Jun  4 16:03:26 2001
@@ -135,6 +135,7 @@
 static request_or_macro *lookup_request(symbol);
 static int get_delim_number(units *, int);
 static int get_delim_number(units *, int, units);
+static symbol get_delim_file_name();
 static int get_line_arg(units *res, int si, charinfo **cp);
 static int read_size(int *);
 static symbol get_delim_name();
@@ -4084,6 +4085,66 @@
   }
 }
 
+static symbol get_delim_file_name()
+{
+  token start;
+  start.next();
+  if (start.eof()) {
+    error("end of input at start of delimited file name");
+    return NULL_SYMBOL;
+  }
+  if (start.newline()) {
+    error("can't delimit file name with a newline");
+    return NULL_SYMBOL;
+  }
+  int start_level = input_stack::get_level();
+  char abuf[ABUF_SIZE];
+  char *buf = abuf;
+  int buf_size = ABUF_SIZE;
+  int i = 0;
+  for (;;) {
+    if (i + 1 > buf_size) {
+      if (buf == abuf) {
+       buf = new char[ABUF_SIZE*2];
+       memcpy(buf, abuf, buf_size);
+       buf_size = ABUF_SIZE*2;
+      }
+      else {
+       char *old_buf = buf;
+       buf = new char[buf_size*2];
+       memcpy(buf, old_buf, buf_size);
+       buf_size *= 2;
+       a_delete old_buf;
+      }
+    }
+    tok.next();
+    if (tok.ch() == ']' && input_stack::get_level() == start_level)
+      break;
+
+    if ((buf[i] = tok.ch()) == 0) {
+      error("missing delimiter (got %1)", tok.description());
+      if (buf != abuf)
+       a_delete buf;
+      return NULL_SYMBOL;
+    }
+    i++;
+  }
+  buf[i] = '\0';
+  if (buf == abuf) {
+    if (i == 0) {
+      error("empty delimited file name");
+      return NULL_SYMBOL;
+    }
+    else
+      return symbol(buf);
+  }
+  else {
+    symbol s(buf);
+    a_delete buf;
+    return s;
+  }
+}
+
 // Implement \R
 
 static void do_register()
@@ -4324,6 +4385,13 @@
 {
   tok.next();
   int c = tok.ch();
+  if (c != '[') {
+    error("`\\O%1' is incorrect syntax, use `\\O[%1]'", char(c), char(c));
+    return 0;
+  }
+  tok.next();
+  c = tok.ch();
+  tok.next();
   switch (c) {
   case '0':
     if (begin_level == 1)
@@ -4344,14 +4412,19 @@
     begin_level--;
     break;
   case '5': {
-    symbol filename = get_delim_name();
+    symbol filename = get_delim_file_name();
+    tok.next();
+    if (filename.is_null()) {
+      error("\\O[5 requires a filename");
+      return 0;
+    }
     if (begin_level == 1)
       return new suppress_node(filename, 'i');
     return 0;
     break;
   }
   default:
-    error("`%1' is an invalid argument to \\O", char(c));
+    error("`%1' is an invalid argument to \\O[", char(c));
   }
   return 0;
 }
--- groff-cvs/tmac/www.tmac     Sat Apr 14 15:25:15 2001
+++ groff-html/tmac/www.tmac    Mon Jun  4 12:13:27 2001
@@ -159,19 +159,19 @@
 .         image \\$2 \\$1.png
 .         bp
 .         tl ''''
-\O0\O1
+\O[0]\O[1]
 .  \}
 .  if \\n[www-html] .begin \{
 .         image \\$2 \\$1.png
-\O0
+\O[0]
 .  \}
 ..
 .\"
 .\" HTML-IMAGE-END - terminates an image for html
 .\"
 .de HTML-IMAGE-END
-.  if r ps4html    \O2\O1\O4
-.  if \\n[www-html] \O2\O1\O4
+.  if r ps4html    \O[2]\O[1]\O[4]
+.  if \\n[www-html] \O[2]\O[1]\O[4]
 ..
 .nr png-no 0
 .\"