Fix input wiping on decryption failure

corretto · Oct 16, 2023 · 3c38a4e · 3c38a4e
1 parent 4c989e1
commit 3c38a4e
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 9 deletions.
diff --git 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
@@ -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);
   }