F-4851 / F-5065: honor caller sz and bound it to the array in CertManager buffer loaders

Author: cconlon

native/com_wolfssl_WolfSSLCertManager.c

@@ -116,8 +116,13 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertManager_CertManagerLoadCABuff
         return BAD_FUNC_ARG;
     }
 
+    /* number of bytes to parse and must fit within the array */
+    if (sz > (jlong)(*jenv)->GetArrayLength(jenv, in)) {
+        return BAD_FUNC_ARG;
+    }
+
     buff = (byte*)(*jenv)->GetByteArrayElements(jenv, in, NULL);
-    buffSz = (*jenv)->GetArrayLength(jenv, in);
+    buffSz = (word32)sz;
 
     if (buff != NULL) {
         ret = wolfSSL_CertManagerLoadCABuffer(cm, buff, buffSz, format);
@@ -159,8 +164,12 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLCertManager_CertManagerVerifyBuff
     if (jenv == NULL || in == NULL || (sz < 0))
         return BAD_FUNC_ARG;
 
+    /* number of bytes to parse and must fit within the array */
+    if (sz > (jlong)(*jenv)->GetArrayLength(jenv, in))
+        return BAD_FUNC_ARG;
+
     buff = (byte*)(*jenv)->GetByteArrayElements(jenv, in, NULL);
-    buffSz = (*jenv)->GetArrayLength(jenv, in);
+    buffSz = (word32)sz;
 
     if (buff != NULL) {
         ret = wolfSSL_CertManagerVerifyBuffer(cm, buff, buffSz, format);

src/java/com/wolfssl/WolfSSLCertManager.java

@@ -116,12 +116,14 @@ public synchronized int CertManagerLoadCA(String f, String d)
      * Load CA into CertManager from byte array
      *
      * @param in byte array holding X.509 certificate to load
-     * @param sz size of input byte array, bytes
+     * @param sz number of bytes to parse from the start of in.
+     *           Must not exceed in.length.
      * @param format format of input certificate, either
      *               WolfSSL.SSL_FILETYPE_PEM (PEM formatted) or
      *               WolfSSL.SSL_FILETYPE_ASN1 (ASN.1/DER).
      *
-     * @return WolfSSL.SSL_SUCCESS on success, negative on error
+     * @return WolfSSL.SSL_SUCCESS on success, negative on error including
+     *         BAD_FUNC_ARG when sz exceeds in.length
      * @throws IllegalStateException WolfSSLContext has been freed
      */
     public synchronized int CertManagerLoadCABuffer(
@@ -225,13 +227,15 @@ public synchronized int CertManagerUnloadCAs()
      * Verify X.509 certificate held in byte array
      *
      * @param in input X.509 certificate as byte array
-     * @param sz size of input certificate array, bytes
+     * @param sz number of bytes to parse from the start of in.
+     *           Must not exceed in.length.
      * @param format format of input certificate, either
      *               WolfSSL.SSL_FILETYPE_PEM (PEM formatted) or
      *               WolfSSL.SSL_FILETYPE_ASN1 (ASN.1/DER).
      *
      * @return WolfSSL.SSL_SUCCESS on successful verification, otherwise
-     *         negative on error.
+     *         negative on error including BAD_FUNC_ARG when sz exceeds
+     *         in.length.
      * @throws IllegalStateException WolfSSLContext has been freed
      */
     public synchronized int CertManagerVerifyBuffer(

src/test/com/wolfssl/test/WolfSSLCertManagerTest.java

@@ -565,6 +565,56 @@ public void testCertManagerLoadCA() throws Exception {
         }
     }
 
+    /* The buffer load/verify calls must parse exactly sz bytes from the start
+     * of the array. A sz beyond the array is rejected instead of being
+     * ignored and the whole backing array parsed. */
+    @Test
+    public void testCertManagerBufferHonorsSz() throws Exception {
+
+        WolfSSLCertManager cm = null;
+
+        try {
+            byte[] caCertPem =
+                Files.readAllBytes(new File(ocspRootCaCert).toPath());
+            byte[] peerPem = Files.readAllBytes(new File(serverCert).toPath());
+
+            cm = new WolfSSLCertManager();
+
+            /* Exact length is the normal, supported call. */
+            assertEquals("exact-length load should succeed",
+                WolfSSL.SSL_SUCCESS,
+                cm.CertManagerLoadCABuffer(caCertPem, caCertPem.length,
+                    WolfSSL.SSL_FILETYPE_PEM));
+
+            assertEquals("load sz past end of array must be rejected",
+                WolfSSL.BAD_FUNC_ARG,
+                cm.CertManagerLoadCABuffer(caCertPem, caCertPem.length + 1,
+                    WolfSSL.SSL_FILETYPE_PEM));
+
+            assertEquals("verify sz past end of array must be rejected",
+                WolfSSL.BAD_FUNC_ARG,
+                cm.CertManagerVerifyBuffer(peerPem, peerPem.length + 1,
+                    WolfSSL.SSL_FILETYPE_PEM));
+
+            /* A shorter sz must parse only the first sz bytes, not the whole
+             * array. Half the PEM is malformed, so the load must fail. The
+             * old code ignored sz and parsed the full array, succeeding. */
+            assertTrue("a shorter sz must not parse the whole array",
+                cm.CertManagerLoadCABuffer(caCertPem, caCertPem.length / 2,
+                    WolfSSL.SSL_FILETYPE_PEM) != WolfSSL.SSL_SUCCESS);
+
+            /* sz of zero parses nothing and must not succeed. */
+            assertTrue("sz of zero must not succeed",
+                cm.CertManagerLoadCABuffer(caCertPem, 0,
+                    WolfSSL.SSL_FILETYPE_PEM) != WolfSSL.SSL_SUCCESS);
+
+        } finally {
+            if (cm != null) {
+                cm.free();
+            }
+        }
+    }
+
     @Test
     public void testCertManagerCheckOCSPResponse()
         throws Exception {