From 4f27d7cdc02e3edfb9d36275341fb8185d6e99ed Mon Sep 17 00:00:00 2001
From: Divy Srivastava <dj.srivastava23@gmail.com>
Date: Mon, 20 Jan 2025 18:16:44 +0530
Subject: [PATCH] fix(ext/node): GCM auth tag check on DechiperIv#final
 (#27733)

---
 ext/node/lib.rs                                  |  1 -
 ext/node/ops/crypto/cipher.rs                    |  5 +++++
 ext/node/ops/crypto/mod.rs                       | 11 -----------
 ext/node/polyfills/internal/crypto/cipher.ts     | 16 +++++++---------
 tests/unit_node/crypto/crypto_cipher_gcm_test.ts | 10 +++++++---
 5 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/ext/node/lib.rs b/ext/node/lib.rs
index 702a01e447..a0efc12657 100644
--- a/ext/node/lib.rs
+++ b/ext/node/lib.rs
@@ -226,7 +226,6 @@ deno_core::extension!(deno_node,
     ops::crypto::op_node_decipheriv_decrypt,
     ops::crypto::op_node_decipheriv_final,
     ops::crypto::op_node_decipheriv_set_aad,
-    ops::crypto::op_node_decipheriv_take,
     ops::crypto::op_node_dh_compute_secret,
     ops::crypto::op_node_diffie_hellman,
     ops::crypto::op_node_ecdh_compute_public_key,
diff --git a/ext/node/ops/crypto/cipher.rs b/ext/node/ops/crypto/cipher.rs
index a12f36e04a..df40ee9ab6 100644
--- a/ext/node/ops/crypto/cipher.rs
+++ b/ext/node/ops/crypto/cipher.rs
@@ -500,6 +500,11 @@ impl Decipher {
     auth_tag: &[u8],
   ) -> Result<(), DecipherError> {
     use Decipher::*;
+
+    if input.is_empty() && !matches!(self, Aes128Gcm(_) | Aes256Gcm(_)) {
+      return Ok(());
+    }
+
     match (self, auto_pad) {
       (Aes128Cbc(decryptor), true) => {
         assert!(input.len() == 16);
diff --git a/ext/node/ops/crypto/mod.rs b/ext/node/ops/crypto/mod.rs
index 8c6b571316..f66a3a1804 100644
--- a/ext/node/ops/crypto/mod.rs
+++ b/ext/node/ops/crypto/mod.rs
@@ -332,17 +332,6 @@ pub fn op_node_decipheriv_decrypt(
   true
 }
 
-#[op2(fast)]
-pub fn op_node_decipheriv_take(
-  state: &mut OpState,
-  #[smi] rid: u32,
-) -> Result<(), cipher::DecipherContextError> {
-  let context = state.resource_table.take::<cipher::DecipherContext>(rid)?;
-  Rc::try_unwrap(context)
-    .map_err(|_| cipher::DecipherContextError::ContextInUse)?;
-  Ok(())
-}
-
 #[op2]
 pub fn op_node_decipheriv_final(
   state: &mut OpState,
diff --git a/ext/node/polyfills/internal/crypto/cipher.ts b/ext/node/polyfills/internal/crypto/cipher.ts
index dd1698f46e..b0593b7c51 100644
--- a/ext/node/polyfills/internal/crypto/cipher.ts
+++ b/ext/node/polyfills/internal/crypto/cipher.ts
@@ -18,7 +18,6 @@ import {
   op_node_decipheriv_decrypt,
   op_node_decipheriv_final,
   op_node_decipheriv_set_aad,
-  op_node_decipheriv_take,
   op_node_private_decrypt,
   op_node_private_encrypt,
   op_node_public_encrypt,
@@ -352,14 +351,6 @@ export class Decipheriv extends Transform implements Cipher {
   }
 
   final(encoding: string = getDefaultEncoding()): Buffer | string {
-    if (!this.#needsBlockCache || this.#cache.cache.byteLength === 0) {
-      op_node_decipheriv_take(this.#context);
-      return encoding === "buffer" ? Buffer.from([]) : "";
-    }
-    if (this.#cache.cache.byteLength != 16) {
-      throw new Error("Invalid final block size");
-    }
-
     let buf = new Buffer(16);
     op_node_decipheriv_final(
       this.#context,
@@ -369,6 +360,13 @@ export class Decipheriv extends Transform implements Cipher {
       this.#authTag || NO_TAG,
     );
 
+    if (!this.#needsBlockCache || this.#cache.cache.byteLength === 0) {
+      return encoding === "buffer" ? Buffer.from([]) : "";
+    }
+    if (this.#cache.cache.byteLength != 16) {
+      throw new Error("Invalid final block size");
+    }
+
     buf = buf.subarray(0, 16 - buf.at(-1)); // Padded in Pkcs7 mode
     return encoding === "buffer" ? buf : buf.toString(encoding);
   }
diff --git a/tests/unit_node/crypto/crypto_cipher_gcm_test.ts b/tests/unit_node/crypto/crypto_cipher_gcm_test.ts
index dd02ee5e32..dfd1208499 100644
--- a/tests/unit_node/crypto/crypto_cipher_gcm_test.ts
+++ b/tests/unit_node/crypto/crypto_cipher_gcm_test.ts
@@ -4,7 +4,7 @@ import crypto from "node:crypto";
 import { Buffer } from "node:buffer";
 import testVectors128 from "./gcmEncryptExtIV128.json" with { type: "json" };
 import testVectors256 from "./gcmEncryptExtIV256.json" with { type: "json" };
-import { assertEquals } from "@std/assert";
+import { assertEquals, assertThrows } from "@std/assert";
 
 const aesGcm = (bits: string, key: Uint8Array) => {
   const ALGO = bits == "128" ? `aes-128-gcm` : `aes-256-gcm`;
@@ -123,7 +123,7 @@ Deno.test({
 // Issue #27441
 // https://github.com/denoland/deno/issues/27441
 Deno.test({
-  name: "aes-256-gcm supports IV of non standard length",
+  name: "aes-256-gcm supports IV of non standard length and auth tag check",
   fn() {
     const decipher = crypto.createDecipheriv(
       "aes-256-gcm",
@@ -136,6 +136,10 @@ Deno.test({
       "utf-8",
     );
     assertEquals(decrypted, "this is a secret");
-    decipher.final();
+    assertThrows(
+      () => decipher.final(),
+      TypeError,
+      "Failed to authenticate data",
+    );
   },
 });