8183591: Incorrect behavior when reading DER value with Integer.MAX_VALUE length

8236983: [TESTBUG] Remove pointless catch block in test/jdk/sun/security/util/DerValue/BadValue.java
Reviewed-by: mbalao

3 files changed, 107 insertions, 17 deletions

diff --git a/src/share/classes/sun/misc/IOUtils.java b/src/share/classes/sun/misc/IOUtils.java
index 3877b03a25..3bc78f0ec8 100644
--- a/src/share/classes/sun/misc/IOUtils.java
+++ b/src/share/classes/sun/misc/IOUtils.java
@@ -54,17 +54,18 @@ public class IOUtils {
      * Read up to {@code length} of bytes from {@code in}
      * until EOF is detected.
      * @param is input stream, must not be null
-     * @param length number of bytes to read, -1 or Integer.MAX_VALUE means
-     *        read as much as possible
+     * @param length number of bytes to read
      * @param readAll if true, an EOFException will be thrown if not enough
-     *        bytes are read. Ignored when length is -1 or Integer.MAX_VALUE
+     *        bytes are read.
      * @return bytes read
      * @throws IOException Any IO error or a premature EOF is detected
      */
     public static byte[] readFully(InputStream is, int length, boolean readAll)
             throws IOException {
+        if (length < 0) {
+            throw new IOException("Invalid length");
+        }
         byte[] output = {};
-        if (length == -1) length = Integer.MAX_VALUE;
         int pos = 0;
         while (pos < length) {
             int bytesToRead;
@@ -78,7 +79,7 @@ public class IOUtils {
             }
             int cc = is.read(output, pos, bytesToRead);
             if (cc < 0) {
-                if (readAll && length != Integer.MAX_VALUE) {
+                if (readAll) {
                     throw new EOFException("Detect premature EOF");
                 } else {
                     if (output.length != pos) {
diff --git a/test/sun/security/provider/DSA/TestMaxLengthDER.java b/test/sun/security/provider/DSA/TestMaxLengthDER.java
new file mode 100644
index 0000000000..a4f447c3c1
--- /dev/null
+++ b/test/sun/security/provider/DSA/TestMaxLengthDER.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8183591
+ * @summary Test decoding of DER length fields containing Integer.MAX_VALUE
+ * @run main TestMaxLengthDER
+ */
+
+import java.io.*;
+import java.math.*;
+import java.security.*;
+import java.security.spec.*;
+
+public class TestMaxLengthDER {
+
+    public static void main(String[] args) throws Exception {
+
+        String message = "Message";
+        Signature sig = Signature.getInstance("SHA256withDSA");
+        KeyPairGenerator kpg = KeyPairGenerator.getInstance("DSA");
+        SecureRandom rnd = new SecureRandom();
+        rnd.setSeed(1);
+        kpg.initialize(2048, rnd);
+        KeyPair kp = kpg.generateKeyPair();
+        sig.initSign(kp.getPrivate());
+        sig.update(message.getBytes());
+        byte[] sigData = sig.sign();
+
+        // Set the length of the second integer to Integer.MAX_VALUE
+        // First copy all the signature data to the correct location
+        int lengthPos = sigData[3] + 5;
+        byte[] modifiedSigData = new byte[sigData.length + 4];
+        System.arraycopy(sigData, 0, modifiedSigData, 0, lengthPos);
+        System.arraycopy(sigData, lengthPos + 1, modifiedSigData,
+            lengthPos + 5, sigData.length - (lengthPos + 1));
+
+        // Increase the length (in bytes) of the sequence to account for
+        // the larger length field
+        modifiedSigData[1] += 4;
+
+        // Modify the length field
+        modifiedSigData[lengthPos] = (byte) 0x84;
+        modifiedSigData[lengthPos + 1] = (byte) 0x7F;
+        modifiedSigData[lengthPos + 2] = (byte) 0xFF;
+        modifiedSigData[lengthPos + 3] = (byte) 0xFF;
+        modifiedSigData[lengthPos + 4] = (byte) 0xFF;
+
+        sig.initVerify(kp.getPublic());
+        sig.update(message.getBytes());
+
+        try {
+            sig.verify(modifiedSigData);
+            throw new RuntimeException("No exception on misencoded signature");
+        } catch (SignatureException ex) {
+            if (ex.getCause() instanceof EOFException) {
+                // this is expected
+            } else {
+                throw ex;
+            }
+        }
+    }
+}
diff --git a/test/sun/security/util/DerValue/BadValue.java b/test/sun/security/util/DerValue/BadValue.java
index ef3a9ef381..772115506c 100644
--- a/test/sun/security/util/DerValue/BadValue.java
+++ b/test/sun/security/util/DerValue/BadValue.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -48,30 +48,35 @@ public class BadValue {
         if (bs.length != 6 || in.available() != 0) {
             throw new Exception("Second read error");
         }
-        // MAX read as much as it can
+        // MAX length results in exception
         in = new ByteArrayInputStream(new byte[10]);
-        bs = IOUtils.readFully(in, Integer.MAX_VALUE, true);
-        if (bs.length != 10 || in.available() != 0) {
-            throw new Exception("Second read error");
+        try {
+            bs = IOUtils.readFully(in, Integer.MAX_VALUE, true);
+            throw new Exception("No exception on MAX_VALUE length");
+        } catch (EOFException ex) {
+            // this is expected
         }
-        // MAX ignore readAll
+        // -1 length results in exception
         in = new ByteArrayInputStream(new byte[10]);
-        bs = IOUtils.readFully(in, Integer.MAX_VALUE, false);
-        if (bs.length != 10 || in.available() != 0) {
-            throw new Exception("Second read error");
+        try {
+            bs = IOUtils.readFully(in, -1, true);
+            throw new Exception("No exception on -1 length");
+        } catch (IOException ex) {
+            // this is expected
         }
+
         // 20>10, readAll means failure
         in = new ByteArrayInputStream(new byte[10]);
         try {
             bs = IOUtils.readFully(in, 20, true);
-            throw new Exception("Third read error");
+            throw new Exception("No exception on EOF");
         } catch (EOFException e) {
             // OK
         }
         int bignum = 10 * 1024 * 1024;
-        bs = IOUtils.readFully(new SuperSlowStream(bignum), -1, true);
+        bs = IOUtils.readFully(new SuperSlowStream(bignum), bignum, true);
         if (bs.length != bignum) {
-            throw new Exception("Fourth read error");
+            throw new Exception("Read returned small array");
         }
 
         // Test DerValue