GHSA-2vgq-wchc-25w7

Summary

decipher.finish() in CBC and ECB mode accepted malformed PKCS#7 padding: only the last byte was checked, against a limit of four blocks, so mismatched padding bytes, a zero count and oversized counts were accepted and up to four blocks of plaintext could be silently removed. Fixed in node-forge 1.4.1.

Details

modes.cbc.prototype.unpad() and modes.ecb.prototype.unpad() in lib/cipherModes.js read the last decrypted byte as the PKCS#7 padding count, rejected it only if it was larger than blockSize << 2 (64 for AES, 32 for DES), and then removed that many bytes. They did not reject a count of 0 or a count larger than one block, and did not check that every padding byte equals the count, as PKCS#7 (RFC 5652 section 6.3) requires. On the Wycheproof AES-CBC-PKCS5 test vectors, 69 of 144 invalid vectors decrypted successfully.

CBC and ECB without a MAC do not provide integrity: an attacker who can modify ciphertext can already make the final block decrypt to any padding. The effect of this bug is that malformed ciphertext is accepted as valid and that oversized counts silently truncate up to four blocks of plaintext. The fix does not protect against padding oracle attacks; applications that decrypt untrusted CBC data need to authenticate it (for example with an HMAC) or use AES-GCM.

The fix requires a count from 1 to the block size and checks every padding byte. Data with other padding schemes that only use the last byte as the count (ANSI X.923, ISO 10126, XML Encryption) now fails to decrypt with default unpadding; set decipher.mode.unpad = false before finish() and remove such padding in application code.

Patches

Fixed in node-forge 1.4.1. CBC and ECB decryption requires valid PKCS#7 padding.

Workarounds

Authenticate CBC/ECB ciphertext before decrypting it, or use AES-GCM. To check padding on 1.4.0, set decipher.mode.unpad = false and validate the padding in application code.

References

• https://www.rfc-editor.org/rfc/rfc5652#section-6.3 (PKCS#7 content-encryption padding)
• https://github.com/C2SP/wycheproof/blob/master/testvectors_v1/aes_cbc_pkcs5_test.json
• https://cwe.mitre.org/data/definitions/354.html (CWE-354: Improper Validation of Integrity Check Value)

Severity

CVSS 3.1: CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:N/I:L/A:N (3.7, Low) CVSS 4.0 (informational): CVSS:4.0/AV:N/AC:L/AT:P/PR:N/UI:N/VC:N/VI:L/VA:N/SC:N/SI:N/SA:N (6.3) CVSS 4.0 rates this Moderate because its integrity weighting is higher; the severity rating follows CVSS 3.1.

Credits

Reported by Oren Yomtov (@orenyomtov, Fireblocks), @zebbern, and Brian Lee (@hibrian827, Georgia Tech SSLab, Team Atlanta), who also suggested the fix.

Proof of concept

var forge = require('node-forge');
// Wycheproof aes_cbc_pkcs5_test tcId 26: zero padding instead of PKCS#7
var d = forge.cipher.createDecipher('AES-CBC',
  forge.util.hexToBytes('db4f3e5e3795cc09a073fa6a81e5a6bc'));
d.start({iv: forge.util.hexToBytes('23468aa734f5f0f19827316ff168e94f')});
d.update(forge.util.createBuffer(
  forge.util.hexToBytes('aa62606a287476777b92d8e4c4e53028')));
console.log(d.finish()); // 1.4.0: true; 1.4.1: false

Directly, unpad() on 1.4.0 also accepts a final block ending in 03 02 01 04 (removing 4 bytes) and a count of 0x20 on 16-byte blocks (removing 32 bytes).

Regression tests are in tests/unit/aes.js ("PKCS#7 padding checks").

Impact

Applications that decrypt AES or DES data in CBC or ECB mode and use the result of decipher.finish() to decide whether the data is valid could accept malformed ciphertext, with up to four blocks of trailing plaintext removed. This also affects PKCS#7 EncryptedData/EnvelopedData decryption, PKCS#12 and encrypted private keys. Because CBC and ECB without a MAC are malleable, the additional capability given to an attacker is small. There is no confidentiality or availability impact.