Use of insecure cryptographic algorithms

[frozolotl]

This crate uses a number of cryptographic algorithms that are no longer considered secure and it uses them in ways that do not guarantee the integrity of the encrypted data.
Furthermore, it is unsound in subtle ways.

If you are a user considering to use this crate in its current state (latest version is 4.0.1), I would highly recommend not doing so.

Alternatives you can use safely.

I would like to give an equivalent and very easy to use alternative, but sadly I can't do so in good conscience. Please do contact a cryptographer if you need good advice on your use-case. It is easy to get things wrong in subtle and catastrophic ways.

If it's not that important, here are some recommendations.
If you simply need to encrypt data with some key, you can use ChaCha20Poly1305.
To generate that key from a password, use Argon2id.
It's important to never reuse ChaCha20Poly1305's nonce for the same key and always generate a new random salt for Argon2id.

I'll henceforth go through each of the variants available in this crate.

MagicCrypt64

At its base, it uses the DES block cipher. It has been practically broken due to its extremely short key lengths for years now.

To support arbitrary key lengths and initialization vector sizes, MagicCrypt64 makes use of CRC64.
This is no key derivation function but a pretty fast error detecting code.
It is not at all suitable as it is used here.

MagicCrypt64 does not use a message authentication code and therefore the encrypted data is not protected against modification.
It's also vulnerable to padding oracle attacks due to its use of PKCS#7 without MAC.

It also contains undefined behavior, where uninitialized memory is used without MaybeUninit: https://github.com/magiclen/rust-magiccrypt/blob/master/src/ciphers/des64.rs#L86.
This would be easy to fix with virtually no performance degradation using Vec::resize_with.

MagicCrypt128

This one uses AES-128-CBC, which is a definite improvement over DES.

However, for its key and IV generation, it uses the MD5 hash function, which is no longer considered secure.

It too does not perform any authentication and uses PKCS#7. It therefore is also vulnerable to padding oracle attacks.

The same bit of undefined behavior previously mentioned is also present here.

MagicCrypt192

This uses AES-192-CBC with the Tiger hash function for the keys.
To my knowledge there aren't any known practical attacks to this hash function, but I would still recommend against using it.
If you are using user-provided passwords as input instead of random data of sufficient length, it is still easy to bruteforce, unlike a password-based key derivation function.

Everything else is the same as in the previous section.

MagicCrypt256

MagicCrypt256 uses AES-256-CBC with SHA-256 for the key hashing.
Everything said in the previous section applies here too.

Recommendations

As I understand, this crate needs to be compatible with the other MagicCrypt libraries and changing the algorithms therefore isn't a good option.
Therefore, I would recommend making it clear in the README and the docs.rs page that this crate ought not to be used. Perhaps similar to the warning sign seen here?
Additionally, archiving this GitHub repository (and of the other MagicCrypt libraries) seems like a good choice.

[Shnatsel]

FYI, there is an open pull request for a security advisory about this issue: rustsec/advisory-db#2181

I am not convinced about this particular use of MD5 being an issue, but I concur with the other issues listed here. So I am inclined to merge it.

I'd also like to add that CBC modes do not provide authentication and open up a lot of attack surface because of that. Modern cryptographic constructions use authenticated encryption to reduce attack surface. That way an attacker can no longer tamper with the cyphertext to try and derive the key by observing decryption attempts.