ROB: Padding issue with AES encryption

[MartinThoma]

Fixes #1221

Credit goes to Alper ahmetoglu for the fix

Co-authored-by: Alper Ahmetoglu ahmetoglu.alper@gmail.com

[codecov]

Codecov Report

Base: 93.73% // Head: 93.69% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (62636e3) compared to base (deb0667).
Patch coverage: 33.33% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1469      +/-   ##
==========================================
- Coverage   93.73%   93.69%   -0.04%     
==========================================
  Files          28       28              
  Lines        5235     5238       +3     
  Branches      997      998       +1     
==========================================
+ Hits         4907     4908       +1     
- Misses        199      200       +1     
- Partials      129      130       +1     

Impacted Files	Coverage Δ
PyPDF2/_encryption.py	92.18% <33.33%> (-0.48%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[MartinThoma]

@exiledkingcc Does this change make sense to you?

[pubpub-zz]

from PDF reference 1.7, §3.5.1:
Strings and streams encrypted with AES use a padding scheme that is described in Internet RFC 2898, PKCS #5: Password-Based Cryptography Specification Version 2.0; see the Bibliography. For an original message length of M, the pad consists of 16 - (M mod 16) bytes whose value is also 16 - (M mod 16). For example, a 9-byte message has a pad of 7 bytes, each with the value 0x07. The pad can be unambiguously removed to determine the original message length when decrypting. Note that the pad is present when M is evenly divisible by 16; it contains 16 bytes of 0x10.

it looks good to me. A test would be welcomed

[pubpub-zz]

from PDF reference 1.7, §3.5.1:
Strings and streams encrypted with AES use a padding scheme that is described in Internet RFC 2898, PKCS #5: Password-Based Cryptography Specification Version 2.0; see the Bibliography. For an original message length of M, the pad consists of 16 - (M mod 16) bytes whose value is also 16 - (M mod 16). For example, a 9-byte message has a pad of 7 bytes, each with the value 0x07. The pad can be unambiguously removed to determine the original message length when decrypting. Note that the pad is present when M is evenly divisible by 16; it contains 16 bytes of 0x10.

it looks good to me. Just surprise it has not been detected earlier : A test would be welcomed

[exiledkingcc]

i'think the PDF file maybe corrupted.
when encrypting, the data should be padded, after encryption, the length of result is always a multiple of 16.
so, decryption needs NO padding.

[MartinThoma]

@exiledkingcc it shouldn't need it, I thought so as well.

But would adding the padding here ever break a non-broken pdf? Maybe it's worth including it for robustness