Release 10.73.41

git-svn-id: http://svn.code.sf.net/p/netpbm/code/super_stable@4438 9d0c8265-081b-0410-96cb-a4ca84ce46f8

5 files changed, 324 insertions, 65 deletions

diff --git a/converter/ppm/ppmtoicr.c b/converter/ppm/ppmtoicr.c
index 3c8be421..32128fef 100644
--- a/converter/ppm/ppmtoicr.c
+++ b/converter/ppm/ppmtoicr.c
@@ -12,11 +12,142 @@
 
 #include <stdbool.h>
 #include <assert.h>
+#include <string.h>
 #include "ppm.h"
+#include "pm_c_util.h"
+#include "shhopt.h"
+#include "mallocvar.h"
+#include "nstring.h"
 
 #define MAXCOLORCT 256
 #define CLUTCOLORCT 768
 
+/* The following are arbitrary limits.  We could not find an official
+   format specification for this.
+*/
+#define MAXSIZE 32767
+#define MAXDISP 1024
+#define MAXNAMELEN 80
+
+
+struct CmdlineInfo {
+    /* All the information the user supplied in the command line,
+       in a form easy for the program to use.
+    */
+    const char * inputFilename;
+    const char * windowname;  /* NULL means not specified */
+    unsigned int expand;
+    unsigned int display;
+};
+
+
+
+
+static void
+parseCommandLine(int argc, const char ** argv,
+                 struct CmdlineInfo * const cmdlineP) {
+/*----------------------------------------------------------------------------
+   Note that the file spec array we return is stored in the storage that
+   was passed to us as the argv array.
+-----------------------------------------------------------------------------*/
+    optEntry *option_def;
+        /* Instructions to OptParseOptions3 on how to parse our options.
+         */
+    optStruct3 opt;
+
+    unsigned int option_def_index;
+    unsigned int windowNameSpec;
+    unsigned int rleSpec;
+    unsigned int expandSpec;
+    unsigned int displaySpec;
+
+    MALLOCARRAY(option_def, 100);
+
+    option_def_index = 0;   /* incremented by OPTENTRY */
+
+    OPTENT3(0,   "windowname",  OPT_STRING, &cmdlineP->windowname,
+            &windowNameSpec, 0);
+    OPTENT3(0,   "expand",      OPT_UINT,   &cmdlineP->expand,
+            &expandSpec,     0);
+    OPTENT3(0,   "display",     OPT_UINT,   &cmdlineP->display,
+            &displaySpec,    0);
+    OPTENT3(0,   "rle",         OPT_FLAG,   NULL,
+            &rleSpec,        0);
+
+    opt.opt_table = option_def;
+    opt.short_allowed = FALSE;  /* We have no short (old-fashioned) options */
+    opt.allowNegNum = FALSE;  /* We may have parms that are negative numbers */
+
+    pm_optParseOptions3(&argc, (char **)argv, opt, sizeof(opt), 0);
+        /* Uses and sets argc, argv, and some of *cmdlineP and others. */
+
+    if (!expandSpec)
+        cmdlineP->expand = 1;
+
+    if (!displaySpec)
+        cmdlineP->display = 0;
+
+    if (rleSpec)
+        pm_error("The -rle command line option no longer exists.");
+
+    if (cmdlineP->expand == 0)
+        pm_error("-expand value must be positive.");
+
+    if (cmdlineP->display > MAXDISP)
+        pm_error("-display value is too large.  Maximum is %u", MAXDISP);
+
+    if (argc-1 < 1) {
+        cmdlineP->inputFilename = "-";
+    } else
+        cmdlineP->inputFilename = argv[1];
+
+    if (argc-1 > 1)
+        pm_error("Program takes zero or one argument (filename).  You "
+                 "specified %u", argc-1);
+
+    if (windowNameSpec) {
+        if (strlen(cmdlineP->windowname) > MAXNAMELEN)
+            pm_error("-windowname value is too long.  (max %u chars)",
+                      MAXNAMELEN);
+        {
+            unsigned int i;
+            for (i = 0; cmdlineP->windowname[i]; ++i) {
+                if (!isprint (cmdlineP->windowname[i]))
+                    pm_error("-window option contains nonprintable character");
+                if (cmdlineP->windowname[i] =='^') {
+                    /* '^' terminates the window name string in ICR */
+                    pm_error("-windowname option value '%s' contains "
+                             "disallowed '^' character.",
+                             cmdlineP->windowname);
+                }
+            }
+        }
+    } else
+        cmdlineP->windowname = NULL;
+}
+
+
+
+static void
+validateComputableSize(unsigned int const cols,
+                       unsigned int const rows,
+                       unsigned int const expand) {
+/*----------------------------------------------------------------------------
+  We don't have any information on what the limit for these values should be.
+
+  The ICR protocol was used around 1990 when PC main memory was measured in
+  megabytes.
+-----------------------------------------------------------------------------*/
+
+    if (cols > MAXSIZE / expand)
+        pm_error("image width (%f) too large to be processed",
+                 (float) cols * expand);
+    if (rows > MAXSIZE / expand)
+        pm_error("image height (%f) too large to be processed",
+                 (float) rows * expand);
+}
+
+
 
 
 static void
@@ -159,71 +290,60 @@ sendOutPicture(pixel **        const pixels,
 
 
 
-int
+static const char *
+windowNameFmFileName(const char * const fileName) {
+
+    /* Use the input file name, with unprintable characters and '^'
+       replaced with '.'.  '^' terminates the window name string in
+       the output file.  Truncate if necessary.
+    */
+    char * windowName;  /* malloced */
+    unsigned int i;
+
+    windowName = malloc(MAXNAMELEN+1);
+    if (!windowName)
+        pm_error("Failed to get %u bytes of memory for window name "
+                 "buffer", MAXNAMELEN+1);
+
+    for (i = 0; i < MAXNAMELEN && fileName[i]; ++i) {
+        const char thisChar = fileName[i];
+
+        if (!isprint(thisChar) || thisChar =='^')
+            windowName[i] = '.';
+        else
+            windowName[i] = thisChar;
+    }
+    windowName[i] = '\0';
+
+    return windowName;
+}
+
+
+            int
 main(int argc, const char ** const argv) {
 
     FILE * ifP;
     int rows, cols;
     int colorCt;
-    int argn;
     unsigned int bitsPerPixel;
     pixval maxval;
     colorhist_vector chv;
     char rgb[CLUTCOLORCT];
-    const char * windowName;
-    int display, expand;
-    int winflag;
-    const char* const usage = "[-windowname windowname] [-expand expand] [-display display] [ppmfile]";
-    pixel** pixels;
+    pixel ** pixels;
     colorhash_table cht;
+    struct CmdlineInfo cmdline;
+    const char * windowName;
 
     pm_proginit(&argc, argv);
 
-    argn = 1;
-    windowName = "untitled";
-    winflag = 0;
-    expand = 1;
-    display = 0;
-
-    while ( argn < argc && argv[argn][0] == '-' && argv[argn][1] != '\0' )
-    {
-        if ( pm_keymatch(argv[argn],"-windowname",2) && argn + 1 < argc )
-        {
-            ++argn;
-            windowName = argv[argn];
-            winflag = 1;
-        }
-        else if ( pm_keymatch(argv[argn],"-expand",2) && argn + 1 < argc )
-        {
-            ++argn;
-            if ( sscanf( argv[argn], "%d",&expand ) != 1 )
-                pm_usage( usage );
-        }
-        else if ( pm_keymatch(argv[argn],"-display",2) && argn + 1 < argc )
-        {
-            ++argn;
-            if ( sscanf( argv[argn], "%d",&display ) != 1 )
-                pm_usage( usage );
-        }
-        else
-            pm_usage( usage );
-    }
-
-    if ( argn < argc )
-    {
-        ifP = pm_openr( argv[argn] );
-        if ( ! winflag )
-            windowName = argv[argn];
-        ++argn;
-    }
-    else
-        ifP = stdin;
+    parseCommandLine(argc, argv, &cmdline);
 
-    if ( argn != argc )
-        pm_usage( usage );
+    ifP = pm_openr(cmdline.inputFilename);
 
     pixels = ppm_readppm(ifP, &cols, &rows, &maxval);
 
+    validateComputableSize(cols, rows, cmdline.expand);
+
     pm_close(ifP);
 
     /* Figure out the colormap. */
@@ -245,17 +365,28 @@ main(int argc, const char ** const argv) {
     /************** Create a new window using ICR protocol *********/
     /* Format is "ESC^W;left;top;width;height;display;windowname"  */
 
-    pm_message("Creating window %s ...", windowName);
+    if (cmdline.windowname)
+        windowName = pm_strdup(cmdline.windowname);
+    else {
+        if (streq(cmdline.inputFilename, "-"))
+            windowName = pm_strdup("untitled");
+        else
+            windowName = windowNameFmFileName(cmdline.inputFilename);
+    }
+    pm_message("Creating window '%s' ...", windowName);
 
     printf("\033^W;%d;%d;%d;%d;%d;%s^",
-           0, 0, cols * expand, rows * expand, display, windowName);
+           0, 0, cols * cmdline.expand, rows * cmdline.expand,
+                 cmdline.display, windowName);
     fflush(stdout);
 
     /****************** Download the colormap.  ********************/
 
     downloadColormap(rgb, windowName);
 
-    sendOutPicture(pixels, rows, cols, cht, expand, windowName);
+    sendOutPicture(pixels, rows, cols, cht, cmdline.expand, windowName);
+
+    pm_strfree(windowName);
 
     return 0;
 }
diff --git a/doc/HISTORY b/doc/HISTORY
index 169eac30..8a458685 100644
--- a/doc/HISTORY
+++ b/doc/HISTORY
@@ -4,6 +4,25 @@ Netpbm.
 CHANGE HISTORY 
 --------------
 
+22.09.24 BJH  Release 10.73.41
+
+              pnmindex: fix shell injection vulnerabilities.  Broken since
+              Netpbm 10.28 (June 2005).
+
+              ppmtoicr: Fix bug: all options cause bogus command line parsing
+              errors.  Always broken.  Ppmtoicr was new in 1991.
+
+              ppmtoicr: Fix arithmetic overflows.
+
+              ppmtoicr: make -rle option issue an error message saying it no
+              longer exists (it did, sort of, before 2015).
+
+              pnmindex: fix shell injection vulnerabilities.  Broken since
+              Netpbm 10.28 (June 2005).
+
+              pnmmargin: fix shell injection vulneraibility.  Always broken
+              (Program was added in primordial Netpbm in 1990).
+
 22.06.24 BJH  Release 10.73.40
 
               palmtopnm: Fix failure with bogus claim of invalid input on
diff --git a/editor/pnmmargin b/editor/pnmmargin
index 0f57d1d4..46cedbb5 100755
--- a/editor/pnmmargin
+++ b/editor/pnmmargin
@@ -65,6 +65,13 @@ fi
 size="$1"
 shift
 
+case $size in
+    ''|*[!0-9]*)
+        echo "Size argument '$size' is not a whole number"
+        exit 1
+        ;;
+esac
+
 if [ ${2-""} ] ; then
     echo "usage: $0 [-white|-black|-color <colorspec>] <size> [pnmfile]" 1>&2
     exit 1
diff --git a/editor/specialty/pnmindex.c b/editor/specialty/pnmindex.c
index 4ec9edaa..ca72633b 100644
--- a/editor/specialty/pnmindex.c
+++ b/editor/specialty/pnmindex.c
@@ -101,6 +101,71 @@ systemf(const char * const fmt,
         
 
 
+static const char *
+shellQuote(const char * const arg) {
+/*----------------------------------------------------------------------------
+   A string that in a Bourne shell command forms a single token whose value
+   understood by the shell is 'arg'.
+
+   For example,
+
+     'arg'           result
+     --------------  --------------
+
+     hello           'hello'
+     two words       'two words'
+     'Bryan's'       \''Bryan'\''s'\'
+
+   Note that in the last example the result is a concatenation of two
+   single-quoted strings and 3 esaped single quotes outside of those quoted
+   strings.
+
+   You can use this safely to insert an arbitrary string from an untrusted
+   source into a shell command without worrying about it changing the nature
+   of the shell command.
+
+   In newly malloced storage.
+-----------------------------------------------------------------------------*/
+    unsigned int const worstCaseResultLen = 3 * strlen(arg) + 1;
+
+    char * buffer;
+    unsigned int i;
+    unsigned int cursor;
+    bool inquotes;
+
+    MALLOCARRAY_NOFAIL(buffer, worstCaseResultLen);
+
+    for (i = 0, cursor=0, inquotes=false; i < strlen(arg); ++i) {
+        if (arg[i] == '\'') {
+            if (inquotes) {
+                buffer[cursor++] = '\'';  /* Close the quotation */
+                inquotes = false;
+            }
+            assert(!inquotes);
+            buffer[cursor++] = '\\';  /* Add escaped */
+            buffer[cursor++] = '\'';  /*    single quote */
+        } else {
+            if (!inquotes) {
+                buffer[cursor++] = '\'';
+                inquotes = true;
+            }
+            assert(inquotes);
+            buffer[cursor++] = arg[i];
+        }
+    }
+    if (inquotes)
+        buffer[cursor++] = '\'';   /* Close the final quotation */
+
+    buffer[cursor++] = '\0';  /* Terminate string */
+
+    assert(cursor <= worstCaseResultLen);
+
+    return buffer;
+}
+
+
+
+
 static void
 parseCommandLine(int argc, char ** argv, 
                  struct cmdlineInfo * const cmdlineP) {
@@ -194,10 +259,38 @@ freeCmdline(struct cmdlineInfo const cmdline) {
 
 
 static void
+validateTmpdir(const char * const dirNm) {
+/*----------------------------------------------------------------------------
+   Abort program if 'dirNm' contains special characters that would cause
+   trouble if included in a shell command.
+
+   The cleanest thing to do would be to allow any characters in the directory
+   name and just quote and escape the properly every time we include the name
+   of a temporary file in a shell command, but we're too lazy for that.
+-----------------------------------------------------------------------------*/
+    if (
+        strchr(dirNm, '\\') ||
+        strchr(dirNm, '\'') ||
+        strchr(dirNm, ' ') ||
+        strchr(dirNm, '"') ||
+        strchr(dirNm, '$') ||
+        strchr(dirNm, '`') ||
+        strchr(dirNm, '!')
+        ) {
+        pm_error("TMPDIR environment variable contains a shell special "
+                 "character; this program cannot use that.");
+    }
+}
+
+
+
+static void
 makeTempDir(const char ** const tempDirP) {
 
     const char * const tmpdir = getenv("TMPDIR") ? getenv("TMPDIR") : "/tmp";
 
+    validateTmpdir(tmpdir);
+
     const char * mytmpdir;
     int rc;
 
@@ -247,6 +340,7 @@ makeTitle(const char * const title,
           const char * const tempDir) {
 
     const char * const invertStage = blackBackground ? "| pnminvert " : "";
+    const char * const titleToken  = shellQuote(title);
 
     const char * fileName;
 
@@ -255,12 +349,11 @@ makeTitle(const char * const title,
     /* This quoting is not adequate.  We really should do this without
        a shell at all.
     */
-    systemf("pbmtext \"%s\" "
-            "%s"
-            "> %s", 
-            title, invertStage, fileName);
+    systemf("pbmtext %s %s > %s",
+            titleToken, invertStage, fileName);
 
     pm_strfree(fileName);
+    pm_strfree(titleToken);
 }
 
 
@@ -269,7 +362,11 @@ static void
 copyImage(const char * const inputFileName,
           const char * const outputFileName) {
 
-    systemf("cat %s > %s", inputFileName, outputFileName);
+    const char * const inputFileNmToken = shellQuote(inputFileName);
+
+    systemf("cat %s > %s", inputFileNmToken, outputFileName);
+
+    pm_strfree(inputFileNmToken);
 } 
 
 
@@ -282,6 +379,8 @@ copyScaleQuantImage(const char * const inputFileName,
                     unsigned int const quant,
                     unsigned int const colors) {
 
+    const char * const inputFileNmToken = shellQuote(inputFileName);
+
     const char * scaleCommand;
 
     switch (PNM_FORMAT_TYPE(format)) {
@@ -289,13 +388,13 @@ copyScaleQuantImage(const char * const inputFileName,
         pm_asprintf(&scaleCommand, 
                     "pamscale -quiet -xysize %u %u %s "
                     "| pgmtopbm > %s",
-                    size, size, inputFileName, outputFileName);
+                    size, size, inputFileNmToken, outputFileName);
         break;
         
     case PGM_TYPE:
         pm_asprintf(&scaleCommand, 
                     "pamscale -quiet -xysize %u %u %s >%s",
-                    size, size, inputFileName, outputFileName);
+                    size, size, inputFileNmToken, outputFileName);
         break;
         
     case PPM_TYPE:
@@ -303,11 +402,11 @@ copyScaleQuantImage(const char * const inputFileName,
             pm_asprintf(&scaleCommand, 
                         "pamscale -quiet -xysize %u %u %s "
                         "| pnmquant -quiet %u > %s",
-                        size, size, inputFileName, colors, outputFileName);
+                        size, size, inputFileNmToken, colors, outputFileName);
         else
             pm_asprintf(&scaleCommand, 
                         "pamscale -quiet -xysize %u %u %s >%s",
-                        size, size, inputFileName, outputFileName);
+                        size, size, inputFileNmToken, outputFileName);
         break;
     default:
         pm_error("Unrecognized Netpbm format: %d", format);
@@ -316,6 +415,7 @@ copyScaleQuantImage(const char * const inputFileName,
     systemf("%s", scaleCommand);
 
     pm_strfree(scaleCommand);
+    pm_strfree(inputFileNmToken);
 }
 
 
@@ -388,14 +488,16 @@ makeImageFile(const char * const thumbnailFileName,
               const char * const outputFileName) {
 
     const char * const blackWhiteOpt = blackBackground ? "-black" : "-white";
-    const char * const invertStage = blackBackground ? "| pnminvert " : "";
+    const char * const invertStage   = blackBackground ? "| pnminvert " : "";
+    const char * inputFileNmToken    = shellQuote(inputFileName);
 
-    systemf("pbmtext \"%s\" "
-            "%s"
+    systemf("pbmtext %s %s"
             "| pnmcat %s -topbottom %s - "
             "> %s",
-            inputFileName, invertStage, blackWhiteOpt, 
+            inputFileNmToken, invertStage, blackWhiteOpt,
             thumbnailFileName, outputFileName);
+
+    pm_strfree(inputFileNmToken);
 }    
 
 
diff --git a/version.mk b/version.mk
index c0b61d11..afebad44 100644
--- a/version.mk
+++ b/version.mk
@@ -1,3 +1,3 @@
 NETPBM_MAJOR_RELEASE = 10
 NETPBM_MINOR_RELEASE = 73
-NETPBM_POINT_RELEASE = 40
+NETPBM_POINT_RELEASE = 41