Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8321156

Improve the handling of invalid UTF-8 byte sequences for ZipInputStream::getNextEntry and ZipFile::getComment

XMLWordPrintable

    • b12
    • generic
    • generic
    • Verified

      A DESCRIPTION OF THE PROBLEM :
      A bug causes uncaught exceptions when when handling ZIP files with certain entry names, entry comments or ZIP file comments. For ZipInputStream/JarInputStream, this only happens with entry names since these classes don’t support entry comments, directory (CEN) names or ZIP file comments. For ZipFile, certain entry comments and ZIP file comments can throw unexpected exceptions, but not entry names. ZipFS provider was not tested.

      This happens on Java 8, 11, 7 and 21, however different versions of the JDK return different results with some tests cases working in some versions but not others.

      The root cause of these bugs is the use of various functions in the ZipCoder class used to decode UTF8 content in ZIP file entry names, entry comments and ZIP file comments; specifically toStringUTF8(). This eventually calls the newStringUTF8NoRepl internal function in StringCoding.java, where an exception is eventually thrown but is never caught or handled. This function is also callable as a public System.newStringUTF8NoRepl() function.

      This is called from the readLOC() method of ZipInputStream.java on lines 301-303 (JDK11) when an entry’s name is parsed. Similarly, this is also called in ZipFile.java, lines 313-321 (JDK11) and 692-698 (JDK11); when comments are being parsed for entries and the entire file:

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      See attached test case

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      No uncaught exceptions
      ACTUAL -
      Getting uncaught exceptions, primarily "java.lang.IllegalArgumentException"

      ---------- BEGIN SOURCE ----------
      import java.io.*;
      import java.util.jar.JarFile;
      import java.util.jar.JarInputStream;
      import java.util.zip.*;

      public class ZipPocCombined {
          public static void main(String[] args) {
              testZipStreams();

              ByteArrayOutputStream inMemoryData = new ByteArrayOutputStream();
              try {
                  ZipOutputStream inMemoryZip = new ZipOutputStream(inMemoryData);
                  inMemoryZip.setComment("zipcomment");
                  ZipEntry entry = new ZipEntry("foo.bar");
                  entry.setComment("entrycomment");
                  inMemoryZip.putNextEntry(entry);
                  inMemoryZip.write(new byte[1]);
                  inMemoryZip.closeEntry();
                  inMemoryZip.close();
              } catch (IOException e) {
              }

              System.out.println("ZipFile - Change local entry name - will pass");
              testZipFile(inMemoryData.toByteArray(), 34, false);
              System.out.println("\nJarFile - Change local entry name - will pass");
              testZipFile(inMemoryData.toByteArray(), 34, true);

              System.out.println("\nZipFile - Change directory entry name - will pass");
              testZipFile(inMemoryData.toByteArray(), 105, false);
              System.out.println("\nJarFile - Change directory entry name - will pass");
              testZipFile(inMemoryData.toByteArray(), 105, true);

              System.out.println("\nZipFile - Change directory entry comment - will fail");
              testZipFile(inMemoryData.toByteArray(), 115, false);
              System.out.println("\nJarFile - Change directory entry comment - will fail");
              testZipFile(inMemoryData.toByteArray(), 115, true);

              System.out.println("\nZipFile - Change zip file comment - will fail");
              testZipFile(inMemoryData.toByteArray(), 145, false);
              System.out.println("\nJarFile - Change zip file comment - will fail");
              testZipFile(inMemoryData.toByteArray(), 145, true);
          }

          static void testZipFile(byte[] data, int position, boolean useJar) {
              data[position] = (byte)0xff;
              File file = null;
              try {
                  file = File.createTempFile("zipfilepoc", ".zip");
                  FileOutputStream os = new FileOutputStream(file);
                  os.write(data);
                  os.close();

                  ZipFile zf = useJar ? new JarFile(file) : new ZipFile(file);
                  zf.getComment();
                  zf.entries().nextElement();
              } catch (IOException e) {
              } catch (IllegalArgumentException e) {
                  e.printStackTrace(System.out);
              }

              if (file != null) {
                  file.deleteOnExit();
              }
          }

          public static void testZipStreams() {
              ByteArrayOutputStream inMemoryData = new ByteArrayOutputStream();
              try {
                  ZipOutputStream inMemoryZip = new ZipOutputStream(inMemoryData);
                  ZipEntry entry = new ZipEntry("foo.bar");
                  inMemoryZip.putNextEntry(entry);
                  inMemoryZip.write(new byte[1]);
                  inMemoryZip.closeEntry();
                  inMemoryZip.close();
              } catch (IOException e) {
              }

              // Change a byte in the filename entry
              byte[] data = inMemoryData.toByteArray();
              data[34] = (byte)0xff;

              System.out.println("\n---- Testing ZipInputStream... ----");
              try {
                  ZipInputStream is = new ZipInputStream(new ByteArrayInputStream(data));
                  is.getNextEntry();
              } catch (IllegalArgumentException|IOException e) {
                  e.printStackTrace(System.out);
              }

              System.out.println("\n---- Testing JarInputStream... ----");
              try {
                  JarInputStream is = new JarInputStream(new ByteArrayInputStream(data));
                  is.getNextEntry();
              } catch (IllegalArgumentException|IOException e) {
                  e.printStackTrace(System.out);
              }
          }
      }
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      Wrap calls to ZIP classes in a try/catch block

      FREQUENCY : always


            lancea Lance Andersen
            webbuggrp Webbug Group
            Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated:
              Resolved: