Skip to content

CodeRabbit Suggets - Critical: AES-256-CBC without authentication + “pad/truncate” key derivation is insecure [🔴 Critical] #268

Description

@rupamkairi

⚠️ Potential issue | 🔴 Critical

Critical: AES-256-CBC without authentication + “pad/truncate” key derivation is insecure. Switch to AEAD + strict key handling.
Issues:

  • CBC provides confidentiality but not integrity (token can be tampered with; also padding-oracle style risks depending on error behavior).
  • String(env.UNSUBSCRIBE_ENCRYPTION_KEY) turning undefined into "undefined" creates a predictable key if env is missing.
  • Padding/truncation can reduce entropy and makes key format unclear.
    Strongly recommend AES-256-GCM (or XChaCha20-Poly1305 via libsodium) and require a 32-byte key (e.g., base64).
-import crypto from 'crypto';
+import crypto from 'crypto';
 import {env} from '../../env.mjs';

 export interface UnsubscribeTokenPayload {
   alertMethodId: string;
   userId: string;
   expiresAt: number; // Unix timestamp
 }

 export interface EncryptionConfig {
-  algorithm: 'aes-256-cbc';
+  algorithm: 'aes-256-gcm';
   keyLength: 32; // 256 bits
-  ivLength: 16; // 128 bits
+  ivLength: 12; // 96-bit nonce recommended for GCM
+  authTagLength: 16; // 128-bit tag
 }

 const ENCRYPTION_CONFIG: EncryptionConfig = {
-  algorithm: 'aes-256-cbc',
+  algorithm: 'aes-256-gcm',
   keyLength: 32,
-  ivLength: 16,
+  ivLength: 12,
+  authTagLength: 16,
 };

+function getKey(): Buffer {
+  const raw = env.UNSUBSCRIBE_ENCRYPTION_KEY;
+  if (!raw) throw new Error('UNSUBSCRIBE_ENCRYPTION_KEY is not set');
+  // Expect base64-encoded 32 bytes
+  const key = Buffer.from(raw, 'base64');
+  if (key.length !== ENCRYPTION_CONFIG.keyLength) {
+    throw new Error(
+      `UNSUBSCRIBE_ENCRYPTION_KEY must be base64 for ${ENCRYPTION_CONFIG.keyLength} bytes`,
+    );
+  }
+  return key;
+}
+
 export function encryptUnsubscribeToken(
   payload: UnsubscribeTokenPayload,
 ): string {
   try {
-    // Generate random IV for each encryption
     const iv = crypto.randomBytes(ENCRYPTION_CONFIG.ivLength);
-
-    // Ensure key is exactly 32 bytes
-    const key = Buffer.from(
-      String(env.UNSUBSCRIBE_ENCRYPTION_KEY).padEnd(32, '0').slice(0, 32),
-    );
-
-    // Create cipher
-    const cipher = crypto.createCipheriv(ENCRYPTION_CONFIG.algorithm, key, iv);
-
-    // Encrypt the JSON payload
     const payloadJson = JSON.stringify(payload);
-    let encrypted = cipher.update(payloadJson, 'utf8', 'hex');
-    encrypted += cipher.final('hex');
-
-    // Combine IV + encrypted data
-    const combined = Buffer.concat([iv, Buffer.from(encrypted, 'hex')]);
-
-    // Return URL-safe base64 encoded string
+    const cipher = crypto.createCipheriv(
+      ENCRYPTION_CONFIG.algorithm,
+      getKey(),
+      iv,
+      { authTagLength: ENCRYPTION_CONFIG.authTagLength },
+    );
+    const ciphertext = Buffer.concat([
+      cipher.update(payloadJson, 'utf8'),
+      cipher.final(),
+    ]);
+    const tag = cipher.getAuthTag();
+    // token = iv || tag || ciphertext
+    const combined = Buffer.concat([iv, tag, ciphertext]);
     return combined.toString('base64url');
   } catch (error) {
     throw new Error(`Token encryption failed: ${(error as Error).message}`);
   }
 }

 export function decryptUnsubscribeToken(
   token: string,
 ): UnsubscribeTokenPayload | null {
   try {
-    // Decode from URL-safe base64
     const combined = Buffer.from(token, 'base64url');
-
-    // Extract components
-    const iv = combined.subarray(0, ENCRYPTION_CONFIG.ivLength);
-    const encrypted = combined.subarray(ENCRYPTION_CONFIG.ivLength);
-
-    // Ensure key is exactly 32 bytes
-    const key = Buffer.from(
-      String(env.UNSUBSCRIBE_ENCRYPTION_KEY).padEnd(32, '0').slice(0, 32),
-    );
-
-    // Create decipher
-    const decipher = crypto.createDecipheriv(
-      ENCRYPTION_CONFIG.algorithm,
-      key,
-      iv,
-    );
-
-    // Decrypt
-    let decrypted = decipher.update(encrypted, undefined, 'utf8');
-    decrypted += decipher.final('utf8');
+    const minLen =
+      ENCRYPTION_CONFIG.ivLength +
+      ENCRYPTION_CONFIG.authTagLength +
+      1;
+    if (combined.length < minLen) return null;
+
+    const iv = combined.subarray(0, ENCRYPTION_CONFIG.ivLength);
+    const tag = combined.subarray(
+      ENCRYPTION_CONFIG.ivLength,
+      ENCRYPTION_CONFIG.ivLength + ENCRYPTION_CONFIG.authTagLength,
+    );
+    const ciphertext = combined.subarray(
+      ENCRYPTION_CONFIG.ivLength + ENCRYPTION_CONFIG.authTagLength,
+    );
+
+    const decipher = crypto.createDecipheriv(
+      ENCRYPTION_CONFIG.algorithm,
+      getKey(),
+      iv,
+      { authTagLength: ENCRYPTION_CONFIG.authTagLength },
+    );
+    decipher.setAuthTag(tag);
+    const decrypted = Buffer.concat([
+      decipher.update(ciphertext),
+      decipher.final(),
+    ]).toString('utf8');

     // Parse JSON payload
     const payload = JSON.parse(decrypted) as UnsubscribeTokenPayload;

     // Validate payload structure
     if (!payload.alertMethodId || !payload.userId || !payload.expiresAt) {
       return null;
     }

     return payload;
   } catch (error) {
     // Return null for any decryption or parsing errors
     return null;
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

export interface EncryptionConfig {
  algorithm: 'aes-256-gcm';
  keyLength: 32; // 256 bits
  ivLength: 12; // 96-bit nonce recommended for GCM
  authTagLength: 16; // 128-bit tag
}

const ENCRYPTION_CONFIG: EncryptionConfig = {
  algorithm: 'aes-256-gcm',
  keyLength: 32,
  ivLength: 12,
  authTagLength: 16,
};

function getKey(): Buffer {
  const raw = env.UNSUBSCRIBE_ENCRYPTION_KEY;
  if (!raw) throw new Error('UNSUBSCRIBE_ENCRYPTION_KEY is not set');
  // Expect base64-encoded 32 bytes
  const key = Buffer.from(raw, 'base64');
  if (key.length !== ENCRYPTION_CONFIG.keyLength) {
    throw new Error(
      `UNSUBSCRIBE_ENCRYPTION_KEY must be base64 for ${ENCRYPTION_CONFIG.keyLength} bytes`,
    );
  }
  return key;
}

/**
 * Encrypts an unsubscribe token payload using AES-256-CBC
 * @param payload The payload to encrypt
 * @returns URL-safe base64 encoded encrypted token
 */
export function encryptUnsubscribeToken(
  payload: UnsubscribeTokenPayload,
): string {
  try {
    const iv = crypto.randomBytes(ENCRYPTION_CONFIG.ivLength);
    const payloadJson = JSON.stringify(payload);
    const cipher = crypto.createCipheriv(
      ENCRYPTION_CONFIG.algorithm,
      getKey(),
      iv,
      { authTagLength: ENCRYPTION_CONFIG.authTagLength },
    );
    const ciphertext = Buffer.concat([
      cipher.update(payloadJson, 'utf8'),
      cipher.final(),
    ]);
    const tag = cipher.getAuthTag();
    // token = iv || tag || ciphertext
    const combined = Buffer.concat([iv, tag, ciphertext]);
    return combined.toString('base64url');
  } catch (error) {
    throw new Error(`Token encryption failed: ${(error as Error).message}`);
  }
}

/**
 * Decrypts an unsubscribe token and returns the payload
 * @param token URL-safe base64 encoded encrypted token
 * @returns Decrypted payload or null if decryption fails
 */
export function decryptUnsubscribeToken(
  token: string,
): UnsubscribeTokenPayload | null {
  try {
    const combined = Buffer.from(token, 'base64url');
    const minLen =
      ENCRYPTION_CONFIG.ivLength +
      ENCRYPTION_CONFIG.authTagLength +
      1;
    if (combined.length < minLen) return null;

    const iv = combined.subarray(0, ENCRYPTION_CONFIG.ivLength);
    const tag = combined.subarray(
      ENCRYPTION_CONFIG.ivLength,
      ENCRYPTION_CONFIG.ivLength + ENCRYPTION_CONFIG.authTagLength,
    );
    const ciphertext = combined.subarray(
      ENCRYPTION_CONFIG.ivLength + ENCRYPTION_CONFIG.authTagLength,
    );

    const decipher = crypto.createDecipheriv(
      ENCRYPTION_CONFIG.algorithm,
      getKey(),
      iv,
      { authTagLength: ENCRYPTION_CONFIG.authTagLength },
    );
    decipher.setAuthTag(tag);
    const decrypted = Buffer.concat([
      decipher.update(ciphertext),
      decipher.final(),
    ]).toString('utf8');

    // Parse JSON payload
    const payload = JSON.parse(decrypted) as UnsubscribeTokenPayload;

    // Validate payload structure
    if (!payload.alertMethodId || !payload.userId || !payload.expiresAt) {
      return null;
    }

    return payload;
  } catch (error) {
    // Return null for any decryption or parsing errors
    return null;
  }
}
🤖 Prompt for AI Agents
In apps/server/src/utils/unsubscribe/tokenEncryption.ts around lines 10 to 102,
the current AES-256-CBC implementation with pad/truncate key handling is
insecure; replace with an AEAD scheme (AES-256-GCM) and strict key handling:
require env.UNSUBSCRIBE_ENCRYPTION_KEY to be provided as a base64-encoded
32-byte key (throw a clear error if missing/invalid), decode it to a 32-byte
Buffer without padEnd/slice, use a 12-byte random IV for each encryption, after
cipher.final collect the auth tag via cipher.getAuthTag(), and encode the output
as base64url concatenating iv + ciphertext + tag; for decryption, parse iv,
ciphertext, tag from the decoded token, set the auth tag on the decipher before
finalizing to ensure integrity, and return null only on
decryption/authentication failures (never rely on implicit key falling back to
"undefined" or truncation).

Originally posted by @coderabbitai[bot] in #245 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions