From 598d2d2866a46a19153d039a985805cdbdc10ee4 Mon Sep 17 00:00:00 2001
From: Greg Rubin <ga_rubin@apple.com>
Date: Wed, 27 Sep 2023 15:00:03 -0700
Subject: [PATCH] Fix input wiping on decryption failure

---
 .../corretto/crypto/provider/AesGcmSpi.java   | 15 ++++++--------
 .../crypto/provider/test/AesTest.java         | 20 +++++++++++++++++++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java b/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java
index 466fe0dd..6cab61be 100644
--- a/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java
+++ b/src/com/amazon/corretto/crypto/provider/AesGcmSpi.java
@@ -707,8 +707,8 @@ private int engineDecryptFinal(
                       workingInputArray,
                       workingInputOffset,
                       workingInputLength,
-                      output,
-                      outputOffset,
+                      workingInputArray, // Output
+                      workingInputOffset, // Output Offset
                       tagLength,
                       key,
                       iv,
@@ -730,8 +730,8 @@ private int engineDecryptFinal(
                 workingInputArray,
                 workingInputOffset,
                 workingInputLength,
-                output,
-                outputOffset,
+                workingInputArray, // Output
+                workingInputOffset, // Output Offset
                 tagLength,
                 key,
                 iv,
@@ -747,12 +747,9 @@ private int engineDecryptFinal(
         }
       }
       // Decryption completed successfully.
+      // Copy from working buffer into actual output
+      System.arraycopy(workingInputArray, workingInputOffset, output, outputOffset, outLen);
       return outLen;
-    } catch (AEADBadTagException e) {
-      final int maxFillSize = output.length - outputOffset;
-      Arrays.fill(
-          output, outputOffset, Math.min(maxFillSize, engineGetOutputSize(inputLen)), (byte) 0);
-      throw e;
     } finally {
       stateReset();
     }
diff --git a/tst/com/amazon/corretto/crypto/provider/test/AesTest.java b/tst/com/amazon/corretto/crypto/provider/test/AesTest.java
index 84095f61..26ac0c1d 100644
--- a/tst/com/amazon/corretto/crypto/provider/test/AesTest.java
+++ b/tst/com/amazon/corretto/crypto/provider/test/AesTest.java
@@ -1199,6 +1199,26 @@ public void safeReuse() throws Throwable {
     assertArrayEquals(plaintext2, c.doFinal(ciphertext2));
   }
 
+  // When decrypting a ciphertext in-place (which is allowed), we should not modify the input buffer
+  // when the encryption fails.
+  @Test
+  public void inplaceShouldNotWipeInput() throws Exception {
+    final GCMParameterSpec spec = new GCMParameterSpec(128, randomIV());
+    final Cipher c = Cipher.getInstance(ALGO_NAME, NATIVE_PROVIDER);
+    c.init(Cipher.ENCRYPT_MODE, key, spec);
+
+    final byte[] properCiphertext = c.doFinal(PLAINTEXT);
+    final byte[] corruptCiphertext = properCiphertext.clone();
+    corruptCiphertext[0] ^= 0x01;
+    final byte[] inputCiphertext = corruptCiphertext.clone();
+    c.init(Cipher.DECRYPT_MODE, key, spec);
+    assertThrows(
+        AEADBadTagException.class,
+        "Tag mismatch!",
+        () -> c.doFinal(inputCiphertext, 0, inputCiphertext.length, inputCiphertext, 0));
+    assertArrayEquals(corruptCiphertext, inputCiphertext);
+  }
+
   private byte[] randomIV() {
     return TestUtil.getRandomBytes(16);
   }