search

HACKERALERT / Picocrypt

A very small, very simple, yet very secure encryption tool.
GNU General Public License v3.0
2.42k stars 145 forks source link

[WEAK KEYS] XORing keyfile digests with each other is bad practice #170

Closed hakavlad closed 1 year ago

hakavlad commented 1 year ago

Using a pair of identical keyfiles leads to the possibility of decryption using any other pair of identical keyfiles. This is a fundamental problem of picocrypt architecture. Fixing it would break backwards compatibility. This problem should at least be documented. An option to solve the problem: detect the use of identical key files and notify the user about the risks.

HACKERALERT commented 1 year ago

Very good observation, thanks for pointing it out! Unfortunately, I didn't catch this bug while designing the keyfile implementation so there's no built-in protection for two identical keyfiles under different filenames yet. Indeed, any fixes involving changing the header format would break compatibility, so I think it's best to just add some safeguards in the code. I don't see this being a common issue for most people and there is already protection against dropping in the same keyfile twice (see here), so a few additional safety checks in the code will be good. As far as I can tell, two identical keyfiles will result in two identical hashes, which when XORed together, will result in a zeroed byte array. So as long as I check to make sure that the final keyfile hash is not a zeroed byte array before XORing it to the Argon2 key, then that should be sufficient to prevent this issue.

HACKERALERT commented 1 year ago

Yeah I've noted this in the changelog so I will fix it in the next release.