[libpng16] Added "tunknown" test and corrected a logic error in

png_handle_unknown() when SAVE support is absent.  Moved the shell test
scripts for contrib/libtests from the libpng top directory to contrib/libtests.
png_handle_unknown() must always read or skip the chunk, if
SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set
a user callback an unknown chunk will not be read, leading to a read
error, which was revealed by the "tunknown" test.

ANNOUNCE

@@ -462,6 +462,13 @@ Version 1.6.0beta28 [August 17, 2012]
     unsafe-to-copy chunks which were dropped before) and eliminates the
     repositioning of vpAg and sTER in pngtest.png by changing pngtest.png
     (so the chunks are where libpng would put them).
+  Added "tunknown" test and corrected a logic error in png_handle_unknown()
+    when SAVE support is absent.  Moved the shell test scripts for
+    contrib/libtests from the libpng top directory to contrib/libtests.
+    png_handle_unknown() must always read or skip the chunk, if
+    SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set
+    a user callback an unknown chunk will not be read, leading to a read
+    error, which was revealed by the "tunknown" test.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

CHANGES

@@ -4213,6 +4213,13 @@ Version 1.6.0beta28 [August 17, 2012]
     unsafe-to-copy chunks which were dropped before) and eliminates the
     repositioning of vpAg and sTER in pngtest.png by changing pngtest.png
     (so the chunks are where libpng would put them).
+  Added "tunknown" test and corrected a logic error in png_handle_unknown()
+    when SAVE support is absent.  Moved the shell test scripts for
+    contrib/libtests from the libpng top directory to contrib/libtests.
+    png_handle_unknown() must always read or skip the chunk, if
+    SAVE_UNKNOWN_CHUNKS is turned off *and* the application does not set
+    a user callback an unknown chunk will not be read, leading to a read
+    error, which was revealed by the "tunknown" test.
 
 Send comments/corrections/commendations to png-mng-implement at lists.sf.net
 (subscription required; visit

configure.ac

@@ -25,7 +25,7 @@ AC_CONFIG_MACRO_DIR([scripts])
 # color-tests requires automake 1.11 or later
 # silent-rules requires automake 1.11 or later
 # dist-xz requires automake 1.11 or later
-# 1.11.1 fixes a security issue in 1.11
+# 1.12.2 fixes a security issue in 1.11.2 and 1.12.1
 AM_INIT_AUTOMAKE([1.12.2 foreign dist-xz dist-bzip2 color-tests silent-rules])
 # was:
 # AM_INIT_AUTOMAKE

contrib/libtests/test-unknown.c

@@ -11,12 +11,9 @@
  *
  * NOTES:
  *   This is a C program that is intended to be linked against libpng.  It
- *   generates bitmaps internally, stores them as PNG files (using the
- *   sequential write code) then reads them back (using the sequential
- *   read code) and validates that the result has the correct data.
- *
- *   The program can be modified and extended to test the correctness of
- *   transformations performed by libpng.
+ *   allows the libpng unknown handling code to be tested by interpreting
+ *   arguemnts to save or discard combinations of chunks.  The program is
+ *   currently just a minimal validation for the built-in libpng facilities.
  */
 
 #include <stdlib.h>
@@ -25,6 +22,8 @@
 
 #include <png.h>
 
+#ifdef PNG_READ_SUPPORTED
+
 #if PNG_LIBPNG_VER < 10500
 /* This deliberately lacks the PNG_CONST. */
 typedef png_byte *png_const_bytep;
@@ -365,11 +364,20 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/)
 
                if (chunk >= 0)
                {
-                  png_byte name[5];
-
-                  memcpy(name, chunk_info[chunk].name, 5);
-                  png_set_keep_unknown_chunks(png_ptr, option, name, 1);
-                  chunk_info[chunk].keep = option;
+                  /* These #if tests have the effect of skipping the arguments
+                   * if SAVE support is unavailable - we can't do a useful test
+                   * in this case, so we just check the arguments!  This could
+                   * be improved in the future by using the read callback.
+                   */
+#                 ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
+                     png_byte name[5];
+
+                     memcpy(name, chunk_info[chunk].name, 5);
+                     png_set_keep_unknown_chunks(png_ptr, option, name, 1);
+                     chunk_info[chunk].keep = option;
+#                 else
+                     (void)option;
+#                 endif
                   continue;
                }
 
@@ -378,8 +386,10 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/)
             case 7: /* default */
                if (memcmp(argv[i], "default", 7) == 0)
                {
-                  png_set_keep_unknown_chunks(png_ptr, option, NULL, 0);
-                  def = option;
+#                 ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
+                     png_set_keep_unknown_chunks(png_ptr, option, NULL, 0);
+                     def = option;
+#                 endif
                   continue;
                }
 
@@ -388,15 +398,21 @@ check(FILE *fp, int argc, const char **argv, png_uint_32p flags/*out*/)
             case 3: /* all */
                if (memcmp(argv[i], "all", 3) == 0)
                {
-                  png_set_keep_unknown_chunks(png_ptr, option, NULL, -1);
-                  def = option;
-
-                  for (chunk = 0; chunk < NINFO; ++chunk)
-                     if (chunk_info[chunk].all)
-                        chunk_info[chunk].keep = option;
+#                 ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
+                     png_set_keep_unknown_chunks(png_ptr, option, NULL, -1);
+                     def = option;
+
+                     for (chunk = 0; chunk < NINFO; ++chunk)
+                        if (chunk_info[chunk].all)
+                           chunk_info[chunk].keep = option;
+#                 endif
                   continue;
                }
 
+               break;
+
+            default: /* some misplaced = */
+
                break;
          }
       }
@@ -499,7 +515,7 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known,
       int i = find_by_flag(flag);
       int keep = chunk_info[i].keep;
       const char *type;
-      const char *error = NULL;
+      const char *errorx = NULL;
 
       if (chunk_info[i].unknown)
       {
@@ -513,34 +529,38 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known,
             type = "UNKNOWN (specified)";
 
          if (flag & known)
-            error = "chunk processed";
+            errorx = "chunk processed";
 
          else switch (keep)
          {
             case PNG_HANDLE_CHUNK_AS_DEFAULT:
                if (flag & unknown)
-                  error = "DEFAULT: unknown chunk saved";
+                  errorx = "DEFAULT: unknown chunk saved";
                break;
 
             case PNG_HANDLE_CHUNK_NEVER:
                if (flag & unknown)
-                  error = "DISCARD: unknown chunk saved";
+                  errorx = "DISCARD: unknown chunk saved";
                break;
 
             case PNG_HANDLE_CHUNK_IF_SAFE:
                if (ancillary(chunk_info[i].name))
                {
                   if (!(flag & unknown))
-                     error = "IF-SAFE: unknown ancillary chunk lost";
+                     errorx = "IF-SAFE: unknown ancillary chunk lost";
                }
 
                else if (flag & unknown)
-                  error = "IF-SAFE: unknown critical chunk saved";
+                  errorx = "IF-SAFE: unknown critical chunk saved";
                break;
 
             case PNG_HANDLE_CHUNK_ALWAYS:
                if (!(flag & unknown))
-                  error = "SAVE: unknown chunk lost";
+                  errorx = "SAVE: unknown chunk lost";
+               break;
+
+            default:
+               errorx = "internal error: bad keep";
                break;
          }
       } /* unknown chunk */
@@ -555,43 +575,47 @@ check_handling(const char *file, int def, png_uint_32 chunks, png_uint_32 known,
              * caught below when checking for inconsistent processing.
              */
             if (keep != PNG_HANDLE_CHUNK_AS_DEFAULT)
-               error = "!DEFAULT: known chunk processed";
+               errorx = "!DEFAULT: known chunk processed";
          }
 
          else /* not processed */ switch (keep)
          {
             case PNG_HANDLE_CHUNK_AS_DEFAULT:
-               error = "DEFAULT: known chunk not processed";
+               errorx = "DEFAULT: known chunk not processed";
                break;
 
             case PNG_HANDLE_CHUNK_NEVER:
                if (flag & unknown)
-                  error = "DISCARD: known chunk saved";
+                  errorx = "DISCARD: known chunk saved";
                break;
 
             case PNG_HANDLE_CHUNK_IF_SAFE:
                if (ancillary(chunk_info[i].name))
                {
                   if (!(flag & unknown))
-                     error = "IF-SAFE: known ancillary chunk lost";
+                     errorx = "IF-SAFE: known ancillary chunk lost";
                }
 
                else if (flag & unknown)
-                  error = "IF-SAFE: known critical chunk saved";
+                  errorx = "IF-SAFE: known critical chunk saved";
                break;
 
             case PNG_HANDLE_CHUNK_ALWAYS:
                if (!(flag & unknown))
-                  error = "SAVE: known chunk lost";
+                  errorx = "SAVE: known chunk lost";
+               break;
+
+            default:
+               errorx = "internal error: bad keep (2)";
                break;
          }
       }
 
-      if (error != NULL)
+      if (errorx != NULL)
       {
          ++error_count;
          fprintf(stderr, "%s: %s %s %s: %s\n",
-            file, type, chunk_info[i].name, position, error);
+            file, type, chunk_info[i].name, position, errorx);
       }
 
       chunks &= ~flag;
@@ -622,6 +646,10 @@ main(int argc, const char **argv)
       exit(2);
    }
 
+#  ifndef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
+      fprintf(stderr,
+         "test-unknown: warning: no 'save' support so arguments ignored\n");
+#  endif
    fp = fopen(argv[argc], "rb");
    if (fp == NULL)
    {
@@ -681,3 +709,13 @@ main(int argc, const char **argv)
 
    return error_count + (strict ? warning_count : 0);
 }
+
+#else
+int
+main(void)
+{
+   fprintf(stderr,
+   " test ignored because libpng was not built with unknown chunk support\n");
+   return 0;
+}
+#endif

contrib/libtests/test-unknown.sh

+#!/bin/sh
+#
+# Run the unknown API tests
+err=0
+image="${srcdir}/pngtest.png"
+#
+# stream 4 is used for the output of the shell, pngtest-log.txt gets all the
+# normal program output.
+exec 4>&1 1>>pngtest-log.txt 2>&1
+
+echo
+echo "============ test-unknown.sh =============="
+
+echo "Running test-unknown.sh" >&4
+
+for tests in \
+ "discard default=discard"\
+ "save default=save"\
+ "if-safe default=if-safe"\
+ "vpAg vpAg=if-safe"\
+ "sTER sTER=if-safe"\
+ "IDAT default=discard IDAT=save"\
+ "sAPI bKGD=save cHRM=save gAMA=save all=discard iCCP=save sBIT=save sRGB=save"
+do
+   set $tests
+   test="$1"
+   shift
+
+   if ./tunknown "$@" "$image" 4>&-
+   then
+      echo "  PASS: test-unknown $test" >&4
+   else
+      echo "  FAIL: test-unknown $test" >&4
+      err=1
+   fi
+done
+
+exit $err

pngrutil.c

@@ -2883,9 +2883,8 @@ png_handle_unknown(png_structrp png_ptr, png_inforp info_ptr,
             keep = PNG_HANDLE_CHUNK_NEVER; /* insufficient memory */
       }
 
-#     ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
-         else
-#     endif
+      else
+         /* Use the SAVE_UNKNOWN_CHUNKS code or skip the chunk */
 #  endif /* PNG_READ_USER_CHUNKS_SUPPORTED */
 
 #  ifdef PNG_SAVE_UNKNOWN_CHUNKS_SUPPORTED
@@ -2913,6 +2912,19 @@ png_handle_unknown(png_structrp png_ptr, png_inforp info_ptr,
 #     ifndef PNG_READ_USER_CHUNKS_SUPPORTED
 #        error no method to support READ_UNKNOWN_CHUNKS
 #     endif
+
+      {
+         /* If here there is no read callback pointer set and no support is
+          * compiled in to just save the unknown chunks, so simply skip this
+          * chunk.  If 'keep' is something other than AS_DEFAULT or NEVER then
+          * the app has erroneously asked for unknown chunk saving when there
+          * is no support.
+          */
+         if (keep > PNG_HANDLE_CHUNK_NEVER)
+            png_app_error(png_ptr, "no unknown chunk support available");
+
+         png_crc_finish(png_ptr, length);
+      }
 #  endif
 
 #  ifdef PNG_STORE_UNKNOWN_CHUNKS_SUPPORTED