Fix ArrayIndexOutOfBoundsExceptions in cert Cache on zero filled array input

The Harmony cert Cache has a long[] where each long is a combination
of a hash and an one-based index into another table containing the
cached values. The cache is searched with Arrays.binarySearch, which
should never find an actual hit, since even a hash hit will look like
a miss since the input hash doesn't contain the one-based index and
entries in the table do. However, this approach has the property that
both hits and misses give the same location in the array, which is
subsequently checked for a real hit/miss with a mask.

However, the hash of a byte array filled with zeroes was zero, which
is found by Arrays.binarySearch in unused slots. Unfortunately, the
code never expects a direct hit, so when it does uses -index-1 to find
the slot to check for hit/miss, it ends up with a negative number,
causing the ArrayIndexOutOfBoundsExceptions.

The solution is ensure that when the hash function returns zero to
simply treat it as a miss. It should not be true for any non-trival
legal input.

Bug: 2753594
Change-Id: I2ee282cc28f22a0ca26da311ae683edf548c67a6

diff --git a/luni/src/main/java/org/apache/harmony/security/provider/cert/Cache.java b/luni/src/main/java/org/apache/harmony/security/provider/cert/Cache.java
index 24fdb2b..f3dd402 100644 (file)
--- a/luni/src/main/java/org/apache/harmony/security/provider/cert/Cache.java
+++ b/luni/src/main/java/org/apache/harmony/security/provider/cert/Cache.java
@@ -157,7 +157,7 @@ public class Cache {
     // END android-removed
 
     // BEGIN android-added
-   /**
+    /**
      * Creates the Cache object of size of 9.
      * @param pref_size specifies how many leading/trailing bytes of object's
      * encoded form will be used for hash computation
@@ -208,6 +208,9 @@ public class Cache {
      * otherwise.
      */
     public boolean contains(long prefix_hash) {
+        if (prefix_hash == 0) {
+            return false;
+        }
         int idx = -1*Arrays.binarySearch(hashes_idx, prefix_hash)-1;
         if (idx == cache_size) {
             return false;
@@ -229,6 +232,9 @@ public class Cache {
      */
     public Object get(long hash, byte[] encoding) {
         hash |= getSuffHash(encoding);
+        if (hash == 0) {
+            return null;
+        }
         int idx = -1*Arrays.binarySearch(hashes_idx, hash)-1;
         if (idx == cache_size) {
             return null;

diff --git a/luni/src/test/java/libcore/java/security/cert/CertificateFactoryTest.java b/luni/src/test/java/libcore/java/security/cert/CertificateFactoryTest.java
new file mode 100644 (file)
index 0000000..e9a91dd
--- /dev/null
+++ b/luni/src/test/java/libcore/java/security/cert/CertificateFactoryTest.java
@@ -0,0 +1,104 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package libcore.java.security.cert;
+
+import java.io.ByteArrayInputStream;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
+import junit.framework.TestCase;
+
+public class CertificateFactoryTest extends TestCase {
+
+    private static final String VALID_CERTIFICATE_PEM =
+            "-----BEGIN CERTIFICATE-----\n"
+            + "MIIDITCCAoqgAwIBAgIQL9+89q6RUm0PmqPfQDQ+mjANBgkqhkiG9w0BAQUFADBM\n"
+            + "MQswCQYDVQQGEwJaQTElMCMGA1UEChMcVGhhd3RlIENvbnN1bHRpbmcgKFB0eSkg\n"
+            + "THRkLjEWMBQGA1UEAxMNVGhhd3RlIFNHQyBDQTAeFw0wOTEyMTgwMDAwMDBaFw0x\n"
+            + "MTEyMTgyMzU5NTlaMGgxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlh\n"
+            + "MRYwFAYDVQQHFA1Nb3VudGFpbiBWaWV3MRMwEQYDVQQKFApHb29nbGUgSW5jMRcw\n"
+            + "FQYDVQQDFA53d3cuZ29vZ2xlLmNvbTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkC\n"
+            + "gYEA6PmGD5D6htffvXImttdEAoN4c9kCKO+IRTn7EOh8rqk41XXGOOsKFQebg+jN\n"
+            + "gtXj9xVoRaELGYW84u+E593y17iYwqG7tcFR39SDAqc9BkJb4SLD3muFXxzW2k6L\n"
+            + "05vuuWciKh0R73mkszeK9P4Y/bz5RiNQl/Os/CRGK1w7t0UCAwEAAaOB5zCB5DAM\n"
+            + "BgNVHRMBAf8EAjAAMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6Ly9jcmwudGhhd3Rl\n"
+            + "LmNvbS9UaGF3dGVTR0NDQS5jcmwwKAYDVR0lBCEwHwYIKwYBBQUHAwEGCCsGAQUF\n"
+            + "BwMCBglghkgBhvhCBAEwcgYIKwYBBQUHAQEEZjBkMCIGCCsGAQUFBzABhhZodHRw\n"
+            + "Oi8vb2NzcC50aGF3dGUuY29tMD4GCCsGAQUFBzAChjJodHRwOi8vd3d3LnRoYXd0\n"
+            + "ZS5jb20vcmVwb3NpdG9yeS9UaGF3dGVfU0dDX0NBLmNydDANBgkqhkiG9w0BAQUF\n"
+            + "AAOBgQCfQ89bxFApsb/isJr/aiEdLRLDLE5a+RLizrmCUi3nHX4adpaQedEkUjh5\n"
+            + "u2ONgJd8IyAPkU0Wueru9G2Jysa9zCRo1kNbzipYvzwY4OA8Ys+WAi0oR1A04Se6\n"
+            + "z5nRUP8pJcA2NhUzUnC+MY+f6H/nEQyNv4SgQhqAibAxWEEHXw==\n"
+            + "-----END CERTIFICATE-----\n";
+
+    private static final String INVALID_CERTIFICATE_PEM =
+            "-----BEGIN CERTIFICATE-----\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA\n"
+            + "AAAAAAAA\n"
+            + "-----END CERTIFICATE-----";
+
+    public void test_generateCertificate() throws Exception {
+        test_generateCertificate(CertificateFactory.getInstance("X509"));
+        test_generateCertificate(CertificateFactory.getInstance("X509", "BC"));
+        test_generateCertificate(CertificateFactory.getInstance("X509", "DRLCertFactory"));
+    }
+    private void test_generateCertificate(CertificateFactory cf) throws Exception {
+        byte[] valid = VALID_CERTIFICATE_PEM.getBytes();
+        Certificate c = cf.generateCertificate(new ByteArrayInputStream(valid));
+        assertNotNull(c);
+
+        try {
+            byte[] invalid = INVALID_CERTIFICATE_PEM.getBytes();
+            cf.generateCertificate(new ByteArrayInputStream(invalid));
+            fail();
+        } catch (CertificateException expected) {
+        }
+    }
+
+}