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

Fix PDF decryption issue for some empty password files #1079

Merged

Conversation

micahsnyder
Copy link
Contributor

Some PDF's with an empty password can't be decrypted. Investigation found that the problem is a strlen check to prevent an overflow rather than passing down the actual length of the allocated field.

Specifically, the UE buffer may have NULL values in it, so a strlen check will claim the field is shorter than it is and then later checks fail because the length is the wrong size.

While at it, I improved code comments on the function reading dictionary key-value strings and switched a flag use a bool rather than an int.

Some PDF's with an empty password can't be decrypted. Investigation
found that the problem is a strlen check to prevent an overflow rather
than passing down the actual length of the allocated field.

Specifically, the UE buffer may have NULL values in it, so a strlen
check will claim the field is shorter than it is and then later checks
fail because the length is the wrong size.

While at it, I improved code comments on the function reading dictionary
key-value strings and switched a flag use a bool rather than an int.
@ragusaa
Copy link
Contributor

ragusaa commented Nov 8, 2023

Code looks fine. Verified more files were extracted with the PR and with MAIN.

@micahsnyder micahsnyder merged commit ebd30d7 into Cisco-Talos:main Nov 21, 2023
23 of 24 checks passed
@micahsnyder micahsnyder deleted the CLAM-2197-pdf-default-password-nulls branch November 21, 2023 22:20
@micahsnyder micahsnyder mentioned this pull request Dec 8, 2023
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.

2 participants