mogilvie / EncryptBundle

Encryption bundle
89 stars 29 forks source link

Plaintext recovery through padding oracle (chosen-ciphertext attack) #54

Closed paragonie-security closed 3 months ago

paragonie-security commented 4 months ago

The Vulnerability

The encryption method is hard-coded to aes-256-cbc:

https://github.com/mogilvie/EncryptBundle/blob/5b445b6fec80d6ef4a1b7b53b699c35c4b1b9dc8/src/Encryptors/OpenSslEncryptor.php#L18

The decryption algorithm is implemented here:

https://github.com/mogilvie/EncryptBundle/blob/5b445b6fec80d6ef4a1b7b53b699c35c4b1b9dc8/src/Encryptors/OpenSslEncryptor.php#L82-L116

This code performs decryption using AES-256-CBC without authentication. By default, OpenSSL uses PKCS#7 padding. This is susceptible to chosen-ciphertext attacks. Specifically, the padding oracle attack.

Exploit Scenario

Alice stores her sensitive data in an app that uses this EncryptBundle. The app stores the encrypted data in a separate database server. The app has a migration utility intended to facilitate key rotation that the developer leaves in the public web root. As a consequence of how this code functions, it returns an HTTP 200 OK response if the decryption succeeds and an HTTP 500 Internal Server Error if an error occurs.

Bob is a hacker that has privileged access to the database server and is aware of the migration utility on the app webserver. The expectation for a library like this is that Bob would be unable to decrypt Alice's records. However, he is able to update her encrypted data (and mark it as a candidate for migration), query Alice's record with the migration utility, and observe if there's a padding error. By doing so, he is able to recover the plaintext for her data without access to the app server's encryption key.

Recommended Mitigation

  1. Use an authenticated encryption method, such as AES-GCM, instead of AES-CBC. Be extra sure to pass the column name (and any other metadata) as the associated data.
mogilvie commented 4 months ago

Added an optional AesGcmEncyptor for implementation if required to dev-master.