From 71331acc9a6ac04ea2370501ea8d1d77de207b71 Mon Sep 17 00:00:00 2001 From: giraffedata Date: Sat, 24 Sep 2022 20:28:21 +0000 Subject: Release 10.73.41 git-svn-id: http://svn.code.sf.net/p/netpbm/code/super_stable@4438 9d0c8265-081b-0410-96cb-a4ca84ce46f8 --- converter/ppm/ppmtoicr.c | 233 ++++++++++++++++++++++++++++++++++---------- doc/HISTORY | 19 ++++ editor/pnmmargin | 7 ++ editor/specialty/pnmindex.c | 128 +++++++++++++++++++++--- version.mk | 2 +- 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 #include +#include #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 ] [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) { @@ -193,11 +258,39 @@ 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 -- cgit 1.4.1