Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more checks to the encryption/decryption logic #399

Merged
merged 3 commits into from
Mar 9, 2025

Conversation

StephanvanSchaik
Copy link
Contributor

This PR adds a few more commits to harden the encryption/decryption logic in lopdf a bit more against invalid inputs.

The first commit ensures that the file encryption length is always 128 bits. It basically sets the Length file to 128 bits and removes the explicit length field in EncryptionVersion::V4.

The second commit adds checks to the AES crypt filters to ensure that the key length is 16 bytes for 128-bit AES and 32 bytes for 256-bit AES and returns errors (instead of potentially causing a panic).

The third commit removes .unwrap() in the AES crypt filters when invalid padding is encountered. This can happen during decryption when the file encryption key is simply not the actual file encryption key that was used to encrypt the streams (e.g., in PDF 2.0 it is possible to store a file encryption key different from the one used to encrypt the streams). This fixes #398. I have also replaced .unwrap() when encrypting which can also theoretically result in padding errors, but shouldn't practically occur (but it just better to return an error rather than causing a panic).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@J-F-Liu J-F-Liu merged commit afa8d9f into J-F-Liu:main Mar 9, 2025
7 checks passed
@StephanvanSchaik StephanvanSchaik deleted the fixes branch March 9, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic called Result::unwrap() on an Err value: UnpadError
2 participants