8028780: JDK KRB5 module throws OutOfMemoryError when CCache is corrupt

Reviewed-by: xuelei

src/share/classes/sun/security/jgss/GSSNameImpl.java

@@ -257,6 +257,10 @@ public class GSSNameImpl implements GSSName {
                               ((0xFF & bytes[pos++]) << 16) |
                               ((0xFF & bytes[pos++]) << 8) |
                               (0xFF & bytes[pos++]));
+        if (pos > bytes.length - mechPortionLen) {
+            throw new GSSExceptionImpl(GSSException.BAD_NAME,
+                    "Exported name mech name is corrupted!");
+        }
         byte[] mechPortion = new byte[mechPortionLen];
         System.arraycopy(bytes, pos, mechPortion, 0, mechPortionLen);
 

src/share/classes/sun/security/krb5/internal/ccache/CCacheInputStream.java

@@ -32,9 +32,11 @@ package sun.security.krb5.internal.ccache;
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Hashtable;
-import java.util.Vector;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.StringTokenizer;
+
+import sun.misc.IOUtils;
 import sun.security.krb5.*;
 import sun.security.krb5.internal.*;
 import sun.security.krb5.internal.util.KrbDataInputStream;
@@ -74,7 +76,6 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
     // this needs to be public for Kinit.
     public Tag readTag() throws IOException {
         char[] buf = new char[1024];
-        byte[] bytes;
         int len;
         int tag = -1;
         int taglen;
@@ -85,7 +86,6 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         if (len < 0) {
             throw new IOException("stop.");
         }
-        bytes = new byte[len + 2];
         if (len > buf.length) {
             throw new IOException("Invalid tag length.");
         }
@@ -101,11 +101,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
             }
             len = len - (4 + taglen);
         }
-        Tag result;
-        if (tag == -1) {
-        }
-        result = new Tag(len, tag, time_offset, usec_offset);
-        return result;
+        return new Tag(len, tag, time_offset, usec_offset);
     }
     /*
      * In file-based credential cache, the realm name is stored as part of
@@ -123,7 +119,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
             type = read(4);
         }
         length = read(4);
-        String[] result = new String[length + 1];
+        List<String> result = new ArrayList<String>();
         /*
          * DCE includes the principal's realm in the count; the new format
          * does not.
@@ -132,21 +128,26 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
             length--;
         for (int i = 0; i <= length; i++) {
             namelength = read(4);
-            if (namelength > MAXNAMELENGTH) {
-                throw new IOException("Invalid name length in principal name.");
-            }
-            byte[] bytes = new byte[namelength];
-            read(bytes, 0, namelength);
-            result[i] = new String(bytes);
+            byte[] bytes = IOUtils.readFully(this, namelength, true);
+            result.add(new String(bytes));
         }
-        if (isRealm(result[0])) {
-            realm = result[0];
-            pname = new String[length];
-            System.arraycopy(result, 1, pname, 0, length);
-            return new PrincipalName(type, pname, new Realm(realm));
+        if (result.isEmpty()) {
+            throw new IOException("No realm or principal");
+        }
+        if (isRealm(result.get(0))) {
+            realm = result.remove(0);
+            if (result.isEmpty()) {
+                throw new IOException("No principal name components");
+            }
+            return new PrincipalName(
+                    type,
+                    result.toArray(new String[result.size()]),
+                    new Realm(realm));
         }
         try {
-            return new PrincipalName(result, type);
+            return new PrincipalName(
+                    result.toArray(new String[result.size()]),
+                    type);
         } catch (RealmException re) {
             return null;
         }
@@ -184,10 +185,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         if (version == KRB5_FCC_FVNO_3)
             read(2); /* keytype recorded twice in fvno 3 */
         keyLen = read(4);
-        byte[] bytes = new byte[keyLen];
-        for (int i = 0; i < keyLen; i++) {
-            bytes[i] = (byte)read();
-        }
+        byte[] bytes = IOUtils.readFully(this, keyLen, true);
         return new EncryptionKey(bytes, keyType, new Integer(version));
     }
 
@@ -211,7 +209,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         int numAddrs, addrType, addrLength;
         numAddrs = read(4);
         if (numAddrs > 0) {
-            HostAddress[] addrs = new HostAddress[numAddrs];
+            List<HostAddress> addrs = new ArrayList<>();
             for (int i = 0; i < numAddrs; i++) {
                 addrType = read(2);
                 addrLength = read(4);
@@ -224,9 +222,9 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
                 byte[] result = new byte[addrLength];
                 for (int j = 0; j < addrLength; j++)
                     result[j] = (byte)read(1);
-                addrs[i] = new HostAddress(addrType, result);
+                addrs.add(new HostAddress(addrType, result));
             }
-            return addrs;
+            return addrs.toArray(new HostAddress[addrs.size()]);
         }
         return null;
     }
@@ -235,18 +233,15 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         int num, adtype, adlength;
         num = read(4);
         if (num > 0) {
-            AuthorizationDataEntry[] auData = new AuthorizationDataEntry[num];
+            List<AuthorizationDataEntry> auData = new ArrayList<>();
             byte[] data = null;
             for (int i = 0; i < num; i++) {
                 adtype = read(2);
                 adlength = read(4);
-                data = new byte[adlength];
-                for (int j = 0; j < adlength; j++) {
-                    data[j] = (byte)read();
-                }
-                auData[i] = new AuthorizationDataEntry(adtype, data);
+                data = IOUtils.readFully(this, adlength, true);
+                auData.add(new AuthorizationDataEntry(adtype, data));
             }
-            return auData;
+            return auData.toArray(new AuthorizationDataEntry[auData.size()]);
         }
         else return null;
     }
@@ -257,9 +252,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         if (length == 0) {
             return null;
         } else {
-            byte[] bytes = new byte[length];
-            read(bytes, 0, length);
-            return bytes;
+            return IOUtils.readFully(this, length, true);
         }
     }
 

src/share/classes/sun/security/krb5/internal/ccache/FileCCacheConstants.java

@@ -49,7 +49,6 @@ public interface FileCCacheConstants {
     public final int KRB5_FCC_FVNO_4 = 0x504;
     public final int FCC_TAG_DELTATIME = 1;
     public final int KRB5_NT_UNKNOWN = 0;
-    public final int MAXNAMELENGTH = 1024;
     public final int TKT_FLG_FORWARDABLE = 0x40000000;
     public final int TKT_FLG_FORWARDED  =  0x20000000;
     public final int TKT_FLG_PROXIABLE   = 0x10000000;

test/sun/security/jgss/GssMemoryIssues.java

+/*
+ * Copyright (c) 2013, 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 8028780
+ * @summary JDK KRB5 module throws OutOfMemoryError when CCache is corrupt
+ * @run main/othervm -Xmx8m GssMemoryIssues
+ */
+
+import org.ietf.jgss.GSSException;
+import org.ietf.jgss.GSSManager;
+import org.ietf.jgss.GSSName;
+
+public class GssMemoryIssues {
+
+    public static void main(String[] argv) throws Exception {
+        GSSManager man = GSSManager.getInstance();
+        String s = "me@REALM";
+        GSSName name = man.createName(s, GSSName.NT_USER_NAME);
+        byte[] exported = name.export();
+        // Offset of the length of the mech name. Length in big endian
+        int lenOffset = exported.length - s.length() - 4;
+        // Make it huge
+        exported[lenOffset] = 0x7f;
+        try {
+            man.createName(exported, GSSName.NT_EXPORT_NAME);
+        } catch (GSSException gsse) {
+            System.out.println(gsse);
+        }
+    }
+}

test/sun/security/krb5/ccache/CorruptedCC.java

+/*
+ * Copyright (c) 2013, 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 8028780
+ * @summary JDK KRB5 module throws OutOfMemoryError when CCache is corrupt
+ * @run main/othervm -Xmx8m CorruptedCC
+ */
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import sun.security.krb5.internal.ccache.CredentialsCache;
+
+public class CorruptedCC {
+    public static void main(String[] args) throws Exception {
+        for (int i=0; i<TimeInCCache.ccache.length; i++) {
+            byte old = TimeInCCache.ccache[i];
+            TimeInCCache.ccache[i] = 0x7f;
+            Files.write(Paths.get("tmpcc"), TimeInCCache.ccache);
+            // The next line will return null for I/O issues. That's OK.
+            CredentialsCache.getInstance("tmpcc");
+            TimeInCCache.ccache[i] = old;
+        }
+    }
+}

test/sun/security/krb5/TimeInCCache.java → test/sun/security/krb5/ccache/TimeInCCache.java

@@ -30,44 +30,56 @@
 import java.io.ByteArrayInputStream;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+
 import sun.security.krb5.internal.ccache.CCacheInputStream;
 import sun.security.krb5.internal.ccache.Credentials;
 
 public class TimeInCCache {
-    public static void main(String[] args) throws Exception {
+    // Attention: this field is also used by CorruptedCC.java test
+    public static byte[] ccache;
+
+    static {
         // A trivial cache file, with startdate and renewTill being zero.
         // The endtime is set to sometime in year 2022, so that isValid()
         // will always check starttime.
-        byte[] ccache = new byte[]{
-            5, 4, 0, 12, 0, 1, 0, 8, -1, -1, -1, 19, -1, -2, 89, 51,
-            0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 10, 77, 65, 88, 73,
-            46, 76, 79, 67, 65, 76, 0, 0, 0, 5, 100, 117, 109, 109, 121, 0,
-            0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 10, 77, 65, 88, 73, 46,
-            76, 79, 67, 65, 76, 0, 0, 0, 5, 100, 117, 109, 109, 121, 0, 0,
-            0, 0, 0, 0, 0, 2, 0, 0, 0, 10, 77, 65, 88, 73, 46, 76,
-            79, 67, 65, 76, 0, 0, 0, 6, 107, 114, 98, 116, 103, 116, 0, 0,
-            0, 10, 77, 65, 88, 73, 46, 76, 79, 67, 65, 76, 0, 17, 0, 0,
-            0, 16, -78, -85, -90, -50, -68, 115, 68, 8, -39, -109, 91, 61, -17, -27,
-            -122, -120, 71, 69, 16, -121, 0, 0, 0, 0, 98, 69, 16, -121, 0, 0,
-            0, 0, 0, 64, -32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 1, 0, 97, -127, -3, 48, -127, -6, -96, 3, 2, 1, 5, -95, 12,
-            27, 10, 77, 65, 88, 73, 46, 76, 79, 67, 65, 76, -94, 31, 48, 29,
-            -96, 3, 2, 1, 0, -95, 22, 48, 20, 27, 6, 107, 114, 98, 116, 103,
-            116, 27, 10, 77, 65, 88, 73, 46, 76, 79, 67, 65, 76, -93, -127, -61,
-            48, -127, -64, -96, 3, 2, 1, 17, -95, 3, 2, 1, 1, -94, -127, -77,
-            4, -127, -80, 43, 65, -66, 34, 21, -34, 37, 35, 32, 50, -14, 122, 77,
-            -3, -29, 37, 99, 50, 125, -43, -96, -78, 85, 23, 41, -80, 68, 2, -109,
-            -27, 38, -41, -72, -32, 127, 63, -76, -22, 81, 33, -114, -30, 104, 125, -81,
-            -29, 70, -25, 23, 100, -75, -25, 62, -120, -78, -61, -100, -74, 50, -117, -127,
-            -16, 79, -106, 62, -39, 91, 100, -10, 23, -88, -18, -47, 51, -19, 113, 18,
-            98, -101, 31, 98, 22, -81, 11, -41, -42, 67, 87, 92, -2, 42, -54, 79,
-            49, -90, 43, -37, 90, -102, 125, 62, -88, -77, 100, 102, 23, -57, -51, 38,
-            68, -44, -57, -102, 103, -6, 85, -58, 74, -117, -87, 67, -103, -36, 110, -122,
-            115, 12, 118, -106, -114, -51, 79, 68, 32, -91, -53, -5, -51, 89, 72, 70,
-            123, -12, -95, 9, 40, -30, -117, 74, 77, 38, 91, 126, -82, 17, 98, 98,
-            -49, 78, 36, 36, 103, -76, -100, -23, 118, -92, -8, 80, 103, -23, -98, 56,
-            21, 65, -77, 0, 0, 0, 0
-        };
+        String var =
+            /*0000*/ "05 04 00 0C 00 01 00 08 FF FF FF 13 FF FE 59 33 " +
+            /*0010*/ "00 00 00 01 00 00 00 01 00 00 00 0A 4D 41 58 49 " +
+            /*0020*/ "2E 4C 4F 43 41 4C 00 00 00 05 64 75 6D 6D 79 00 " +
+            /*0030*/ "00 00 01 00 00 00 01 00 00 00 0A 4D 41 58 49 2E " +
+            /*0040*/ "4C 4F 43 41 4C 00 00 00 05 64 75 6D 6D 79 00 00 " +
+            /*0050*/ "00 00 00 00 00 02 00 00 00 0A 4D 41 58 49 2E 4C " +
+            /*0060*/ "4F 43 41 4C 00 00 00 06 6B 72 62 74 67 74 00 00 " +
+            /*0070*/ "00 0A 4D 41 58 49 2E 4C 4F 43 41 4C 00 11 00 00 " +
+            /*0080*/ "00 10 B2 AB A6 CE BC 73 44 08 D9 93 5B 3D EF E5 " +
+            /*0090*/ "86 88 47 45 10 87 00 00 00 00 62 45 10 87 00 00 " +
+            /*00A0*/ "00 00 00 40 E0 00 00 00 00 00 00 00 00 00 00 00 " +
+            /*00B0*/ "00 01 00 61 81 FD 30 81 FA A0 03 02 01 05 A1 0C " +
+            /*00C0*/ "1B 0A 4D 41 58 49 2E 4C 4F 43 41 4C A2 1F 30 1D " +
+            /*00D0*/ "A0 03 02 01 00 A1 16 30 14 1B 06 6B 72 62 74 67 " +
+            /*00E0*/ "74 1B 0A 4D 41 58 49 2E 4C 4F 43 41 4C A3 81 C3 " +
+            /*00F0*/ "30 81 C0 A0 03 02 01 11 A1 03 02 01 01 A2 81 B3 " +
+            /*0100*/ "04 81 B0 2B 41 BE 22 15 DE 25 23 20 32 F2 7A 4D " +
+            /*0110*/ "FD E3 25 63 32 7D D5 A0 B2 55 17 29 B0 44 02 93 " +
+            /*0120*/ "E5 26 D7 B8 E0 7F 3F B4 EA 51 21 8E E2 68 7D AF " +
+            /*0130*/ "E3 46 E7 17 64 B5 E7 3E 88 B2 C3 9C B6 32 8B 81 " +
+            /*0140*/ "F0 4F 96 3E D9 5B 64 F6 17 A8 EE D1 33 ED 71 12 " +
+            /*0150*/ "62 9B 1F 62 16 AF 0B D7 D6 43 57 5C FE 2A CA 4F " +
+            /*0160*/ "31 A6 2B DB 5A 9A 7D 3E A8 B3 64 66 17 C7 CD 26 " +
+            /*0170*/ "44 D4 C7 9A 67 FA 55 C6 4A 8B A9 43 99 DC 6E 86 " +
+            /*0180*/ "73 0C 76 96 8E CD 4F 44 20 A5 CB FB CD 59 48 46 " +
+            /*0190*/ "7B F4 A1 09 28 E2 8B 4A 4D 26 5B 7E AE 11 62 62 " +
+            /*01A0*/ "CF 4E 24 24 67 B4 9C E9 76 A4 F8 50 67 E9 9E 38 " +
+            /*01B0*/ "15 41 B3 00 00 00 00 ";
+        ccache = new byte[var.length()/3];
+        for (int i=0; i<ccache.length; i++) {
+            ccache[i] = Integer.valueOf(var.substring(3*i,3*i+2), 16).byteValue();
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
         System.setProperty("sun.security.krb5.debug", "true");  // test code changes in DEBUG
         CCacheInputStream cis = new CCacheInputStream(new ByteArrayInputStream(ccache));
         cis.readVersion();