Fix use of unset variable - entries in the colornames table don't get set when ppm_readcolornamefile finds the named dictionary file doesn't exist, so returns an empty color dictionary

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

2 files changed, 81 insertions, 27 deletions

diff --git a/doc/HISTORY b/doc/HISTORY
index ab8efb13..b32ab0ae 100644
--- a/doc/HISTORY
+++ b/doc/HISTORY
@@ -40,6 +40,11 @@ not yet  BJH  Release 10.63.00
               middle of the AND mask.  Always broken (program was new in
               Netpbm 9.3 (June 2000)).
 
+              libnetpbm: fix bug: ppm_freecolornames() has wild pointer
+              dereference when the color name table was generated empty
+              because the color dictionary file was not openable.  ppmtoxpm
+              suffers from this.
+
               sparc64 pbmtog3: fix bug that causes crash due to unaligned
               memory access.
 
diff --git a/lib/libppmcolor.c b/lib/libppmcolor.c
index 3e7bf27c..347bab29 100644
--- a/lib/libppmcolor.c
+++ b/lib/libppmcolor.c
@@ -451,6 +451,43 @@ ppm_colorname(const pixel * const colorP,
 
 #define MAXCOLORNAMES 1000u
 
+static const char **
+allocColorNames() {
+
+    const char ** colornames;
+
+    MALLOCARRAY(colornames, MAXCOLORNAMES);
+
+    if (colornames) {
+        unsigned int i;
+        for (i = 0; i < MAXCOLORNAMES; ++i)
+            colornames[i] = NULL;
+    }
+    return colornames;
+}
+
+
+
+static colorhash_table
+allocColorHash(void) {
+
+    colorhash_table cht;
+    jmp_buf jmpbuf;
+    jmp_buf * origJmpbufP;
+
+    if (setjmp(jmpbuf) != 0)
+        cht = NULL;
+    else {
+        pm_setjmpbufsave(&jmpbuf, &origJmpbufP);
+        cht = ppm_alloccolorhash();
+    }
+    pm_setjmpbuf(origJmpbufP);
+
+    return cht;
+}
+
+
+
 static void
 processColorfileEntry(struct colorfile_entry const ce,
                       colorhash_table        const cht,
@@ -525,6 +562,9 @@ readOpenColorFile(FILE *          const colorFileP,
    Read the color dictionary file *colorFileP and add the colors in it
    to colornames[], colors[], and 'cht'.
 
+   colornames[] and colors[] must be allocated with MAXCOLORNAMES entries
+   at entry.
+
    We may add colors to 'cht' even if we fail.
 -----------------------------------------------------------------------------*/
     unsigned int nColorsDone;
@@ -543,12 +583,7 @@ readOpenColorFile(FILE *          const colorFileP,
             processColorfileEntry(ce, cht, colornames, colors,
                                   &nColorsDone, errorP);
     }
-    if (!*errorP) {
-        *nColorsP = nColorsDone;
-        
-        while (nColorsDone < MAXCOLORNAMES)
-            colornames[nColorsDone++] = NULL;
-    }
+    *nColorsP = nColorsDone;
     
     if (*errorP) {
         unsigned int colorIndex;
@@ -560,26 +595,6 @@ readOpenColorFile(FILE *          const colorFileP,
 
 
 
-static colorhash_table
-allocColorHash(void) {
-
-    colorhash_table cht;
-    jmp_buf jmpbuf;
-    jmp_buf * origJmpbufP;
-
-    if (setjmp(jmpbuf) != 0)
-        cht = NULL;
-    else {
-        pm_setjmpbufsave(&jmpbuf, &origJmpbufP);
-        cht = ppm_alloccolorhash();
-    }
-    pm_setjmpbuf(origJmpbufP);
-
-    return cht;
-}
-
-
-
 static void
 readColorFile(const char *    const fileName,
               bool            const mustOpen,
@@ -588,7 +603,20 @@ readColorFile(const char *    const fileName,
               pixel *         const colors,
               colorhash_table const cht,
               const char **   const errorP) {
+/*----------------------------------------------------------------------------
+   Read the color dictionary file named 'fileName' and add the colors in it
+   to colornames[], colors[], and 'cht'.  Return as *nColorsP the number
+   of colors in it.
+
+   If the file is not openable (e.g. not file by that name exists), abort the
+   program if 'mustOpen' is true; otherwise, return values indicating a
+   dictionary with no colors.
 
+   colornames[] and colors[] must be allocated with MAXCOLORNAMES entries
+   at entry.
+
+   We may add colors to 'cht' even if we fail.
+-----------------------------------------------------------------------------*/
     FILE * colorFileP;
 
     openColornameFile(fileName, mustOpen, &colorFileP, errorP);
@@ -621,7 +649,7 @@ readcolordict(const char *      const fileName,
 
     const char ** colornames;
 
-    MALLOCARRAY(colornames, MAXCOLORNAMES);
+    colornames = allocColorNames();
 
     if (colornames == NULL)
         pm_asprintf(errorP, "Unable to allocate space for colorname table.");
@@ -670,7 +698,28 @@ ppm_readcolordict(const char *      const fileName,
                   const char ***    const colornamesP,
                   pixel **          const colorsP,
                   colorhash_table * const chtP) {
+/*----------------------------------------------------------------------------
+   Read the color dictionary from the file named 'fileName'.  If we can't open
+   the file (e.g. because it does not exist), and 'mustOpen' is false, return
+   an empty dictionary (it contains no colors).  But if 'mustOpen' is true,
+   abort the program instead of returning an empty dictionary.
+
+   Return as *nColorsP the number of colors in the dictionary.
 
+   Return as *colornamesP the names of those colors.  *colornamesP is a
+   malloced array that Caller must free with ppm_freecolornames().
+   The first *nColorsP entries are valid; *chtP contains indices into this
+   array.
+
+   Return as *colorsP the colors.  *colorsP is a malloced array of size
+   MAXCOLORS with the first elements filled in and the rest undefined.
+
+   Return as *chtP a color hash table mapping each color in the dictionary
+   to the index into *colornamesP for the name of the color.
+
+   Each of 'nColorsP, 'colornamesP', and 'colorsP' may be null, in which case
+   we do not return the corresponding information (or allocate memory for it).
+-----------------------------------------------------------------------------*/
     colorhash_table cht;
     const char ** colornames;
     pixel * colors;