Merge pull request ibmruntimes#475 from pshipton/avoidoverlay

Prevent AES/CBC native buffer overflow

Author: keithc-ca

closed/src/java.base/share/classes/com/sun/crypto/provider/NativeCipherBlockChaining.java

@@ -24,7 +24,7 @@
  */
 /*
  * ===========================================================================
- * (c) Copyright IBM Corp. 2018, 2023 All Rights Reserved
+ * (c) Copyright IBM Corp. 2018, 2025 All Rights Reserved
  * ===========================================================================
  */
 
@@ -33,6 +33,8 @@
 import java.security.InvalidKeyException;
 import java.security.ProviderException;
 import java.util.ArrayDeque;
+import java.util.Arrays;
+
 import com.sun.crypto.provider.AESCrypt;
 
 import jdk.crypto.jniprovider.NativeCrypto;
@@ -47,7 +49,6 @@
  * native implementation of CBC crypto.
  *
  */
-
 class NativeCipherBlockChaining extends FeedbackCipher  {
 
     protected final static int numContexts = 4096;
@@ -58,6 +59,12 @@ class NativeCipherBlockChaining extends FeedbackCipher  {
     private static final Cleaner contextCleaner;
     private int previousKeyLength = -1;
 
+    /**
+     * OpenSSL requires an additional block size for operations. This will be added
+     * to all calculated output buffer sizes whenever native CBC operations are enabled.
+     */
+    static final int OPENSSL_ENCRYPTION_RESIDUE = 16;
+
     /*
      * Initialize the CBC context.
      */
@@ -104,6 +111,29 @@ public void run() {
         }
     }
 
+    /**
+     * This method checks if there is enough space in the provided output buffer
+     * to accommodate encryption in OpenSSL. OpenSSL requires an extra full block size
+     * for its operations according to the documentation associated with EVP_CipherUpdate
+     * (EVP_EncryptUpdate).
+     *
+     * @param output the original output buffer
+     * @param outputOffset the current offset in the output buffer
+     * @param inputLen the length of the input data
+     * @return a new byte array that can hold the combined output and residue (extra block),
+     * or the original buffer if there is enough space in the output buffer {@code output}
+     */
+    private static byte[] getOptionalLocalOpenSSLOutputBuffer(byte[] output, int outputOffset, int inputLen) {
+        byte[] tmpOutputBuffer;
+        int extraLen = Math.addExact(inputLen, OPENSSL_ENCRYPTION_RESIDUE);
+        if (extraLen > (output.length - outputOffset)) {
+            tmpOutputBuffer = new byte[extraLen];
+        } else {
+            tmpOutputBuffer = output;
+        }
+        return tmpOutputBuffer;
+    }
+
     /*
      * Get CBC context.
      */
@@ -266,7 +296,7 @@ int encrypt(byte[] plain, int plainOffset, int plainLen,
             throw new ProviderException("Internal error in input buffering");
         }
 
-        /**
+        /*
          * OpenSSL doesn't support overlapping buffers, make a copy of plain.
          */
         if (plain == cipher) {
@@ -276,15 +306,34 @@ int encrypt(byte[] plain, int plainOffset, int plainLen,
             plainOffset = 0;
         }
 
+        /*
+         * Determine if our output buffer is big enough for OpenSSL operations. Allocate a new
+         * one if required.
+         */
+        byte[] tmpOutputBuffer = getOptionalLocalOpenSSLOutputBuffer(cipher, cipherOffset, plainLen);
+
         int ret;
         synchronized (this) {
-            ret = nativeCrypto.CBCUpdate(nativeContext, plain, plainOffset,
-                                            plainLen, cipher, cipherOffset);
+            ret = nativeCrypto.CBCUpdate(nativeContext,
+                                         plain,
+                                         plainOffset,
+                                         plainLen,
+                                         tmpOutputBuffer,
+                                         (cipher == tmpOutputBuffer) ? cipherOffset : 0);
         }
         if (ret == -1) {
             throw new ProviderException("Error in Native CipherBlockChaining");
         }
 
+        /*
+         * If a larger output buffer was required for OpenSSL operations then copy back the results
+         * into the callers output buffer.
+         */
+        if (cipher != tmpOutputBuffer) {
+            System.arraycopy(tmpOutputBuffer, 0, cipher, cipherOffset, ret);
+            Arrays.fill(tmpOutputBuffer, (byte)0x00);
+        }
+
         // saving current running state
         System.arraycopy(cipher, cipherOffset+plainLen-blockSize, r, 0, blockSize);
         return ret;
@@ -339,16 +388,39 @@ int encryptFinal(byte[] plain, int plainOffset, int plainLen,
 
         int ret;
 
+        /*
+         * Determine if our output buffer is big enough for OpenSSL operations. Allocate a new
+         * one if required.
+         */
+        byte[] tmpOutputBuffer = getOptionalLocalOpenSSLOutputBuffer(cipher, cipherOffset, plainLen);
+
         synchronized (this) {
             if (plain == cipher) {
-                ret = nativeCrypto.CBCFinalEncrypt(nativeContext, plain.clone(),
-                                                plainOffset, plainLen, cipher, cipherOffset);
+                ret = nativeCrypto.CBCFinalEncrypt(nativeContext,
+                                                   plain.clone(),
+                                                   plainOffset,
+                                                   plainLen,
+                                                   tmpOutputBuffer,
+                                                   (cipher == tmpOutputBuffer) ? cipherOffset : 0);
             } else {
-                ret = nativeCrypto.CBCFinalEncrypt(nativeContext, plain, plainOffset,
-                                                    plainLen, cipher, cipherOffset);
+                ret = nativeCrypto.CBCFinalEncrypt(nativeContext,
+                                                   plain,
+                                                   plainOffset,
+                                                   plainLen,
+                                                   tmpOutputBuffer,
+                                                   (cipher == tmpOutputBuffer) ? cipherOffset : 0);
             }
         }
 
+        /*
+         * If a larger output buffer was required for OpenSSL operations then copy back the results
+         * into the callers output buffer.
+         */
+        if (cipher != tmpOutputBuffer) {
+            System.arraycopy(tmpOutputBuffer, 0, cipher, cipherOffset, ret);
+            Arrays.fill(tmpOutputBuffer, (byte)0x00);
+        }
+
         if (ret == -1) {
             throw new ProviderException("Error in Native CipherBlockChaining");
         }

src/java.base/share/classes/com/sun/crypto/provider/CipherCore.java

@@ -24,7 +24,7 @@
  */
 /*
  * ===========================================================================
- * (c) Copyright IBM Corp. 2018, 2023 All Rights Reserved
+ * (c) Copyright IBM Corp. 2018, 2025 All Rights Reserved
  * ===========================================================================
  */
 
@@ -323,6 +323,20 @@ private int getOutputSizeByOperation(int inputLen, boolean isDoFinal) {
         return totalLen;
     }
 
+    /**
+     * Get appropriate sized local working buffer. An additional block size is added
+     * for operations that will use the NativeCipherBlockChaining cipher.
+     *
+     * @see {@link NativeCipherBlockChaining#getOptionalLocalOpenSSLOutputBuffer(byte[], int, int)
+     */
+    private int getLocalWorkingBufferSize(int inputLen, boolean isDoFinal) {
+        int size = getOutputSizeByOperation(inputLen, isDoFinal);
+        if (cipher instanceof NativeCipherBlockChaining) {
+            size = Math.addExact(size, NativeCipherBlockChaining.OPENSSL_ENCRYPTION_RESIDUE);
+        }
+        return size;
+    }
+
     /**
      * Returns the initialization vector (IV) in a new buffer.
      *
@@ -570,7 +584,7 @@ byte[] update(byte[] input, int inputOffset, int inputLen) {
 
         byte[] output = null;
         try {
-            output = new byte[getOutputSizeByOperation(inputLen, false)];
+            output = new byte[getLocalWorkingBufferSize(inputLen, false)];
             int len = update(input, inputOffset, inputLen, output,
                              0);
             if (len == output.length) {
@@ -748,7 +762,7 @@ int update(byte[] input, int inputOffset, int inputLen, byte[] output,
     byte[] doFinal(byte[] input, int inputOffset, int inputLen)
         throws IllegalBlockSizeException, BadPaddingException {
         try {
-            byte[] output = new byte[getOutputSizeByOperation(inputLen, true)];
+            byte[] output = new byte[getLocalWorkingBufferSize(inputLen, true)];
             int outputOffset = 0;
             int outLen = 0;