From e3b528542ee32335e085608f9800c523737c3d66 Mon Sep 17 00:00:00 2001 From: giraffedata Date: Sat, 24 Sep 2022 20:18:34 +0000 Subject: Release 10.86.35 git-svn-id: http://svn.code.sf.net/p/netpbm/code/stable@4436 9d0c8265-081b-0410-96cb-a4ca84ce46f8 --- doc/HISTORY | 10 +++- editor/pnmmargin | 7 +++ editor/specialty/pnmindex.c | 128 +++++++++++++++++++++++++++++++++++++++----- version.mk | 2 +- 4 files changed, 131 insertions(+), 16 deletions(-) diff --git a/doc/HISTORY b/doc/HISTORY index 20d94d83..1a1fbfaf 100644 --- a/doc/HISTORY +++ b/doc/HISTORY @@ -4,6 +4,14 @@ Netpbm. CHANGE HISTORY -------------- +22.09.24 BJH Release 10.86.35 + + 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.07.17 BJH Release 10.86.34 ppmtoicr: Fix bug: all options cause bogus command line parsing @@ -14,8 +22,6 @@ CHANGE HISTORY ppmtoicr: make -rle option issue an error message saying it no longer exists (it did, sort of, before 2015). -22.04.24 BJH Release 10.86.33 - palmtopnm: Fix failure with bogus claim of invalid input on architectures that do not use two's complement negative numbers. Always broken. (Ability to convert PackBits input was new in diff --git a/editor/pnmmargin b/editor/pnmmargin index 1b5370be..6e70aba3 100755 --- a/editor/pnmmargin +++ b/editor/pnmmargin @@ -67,6 +67,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 0a8e35cc..438fe058 100644 --- a/editor/specialty/pnmindex.c +++ b/editor/specialty/pnmindex.c @@ -102,6 +102,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,11 +259,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; @@ -248,6 +341,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; @@ -256,12 +350,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); } @@ -270,7 +363,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); } @@ -283,6 +380,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)) { @@ -290,13 +389,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: @@ -304,11 +403,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); @@ -317,6 +416,7 @@ copyScaleQuantImage(const char * const inputFileName, systemf("%s", scaleCommand); pm_strfree(scaleCommand); + pm_strfree(inputFileNmToken); } @@ -389,14 +489,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 4f8961fa..bff791a6 100644 --- a/version.mk +++ b/version.mk @@ -1,3 +1,3 @@ NETPBM_MAJOR_RELEASE = 10 NETPBM_MINOR_RELEASE = 86 -NETPBM_POINT_RELEASE = 34 +NETPBM_POINT_RELEASE = 35 -- cgit 1.4.1