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

Multiple security issues with the implementation of AES encryption #12

Open
fegge opened this issue Dec 10, 2024 · 0 comments
Open

Multiple security issues with the implementation of AES encryption #12

fegge opened this issue Dec 10, 2024 · 0 comments

Comments

@fegge
Copy link

fegge commented Dec 10, 2024

The implementation of encryption and decryption provided by this library is not secure.

Using Math.random to generate cryptographic keys as you do here is not secure. The Math.random pseudo-random generator is not meant to be used to generate cryptographic keys, and an attacker can recover the internal state of the generator and all generated keys from just a few outputs. You should be using the platforms underlying cryptographically secure pseudo-random number generator instead.

const generateKey = (length) => {
let key = '';
const hex = '0123456789abcdef';
for (i = 0; i < length; i++) {
key += hex.charAt(Math.floor(Math.random() * 16));
}
return key;
};

You are also generating 256 nibbles (i.e. 128 bytes) of (weak) randomness in the call to generateKey, not 128 or 256 bits which are the key sizes used for AES-128 and AES-256. Moreover, since the key is passed as a string to CryptoJS, it is treated as a password and not an encryption key which is probably not what you want. (CryptoJS seems to be expecting a key size of 8 bytes, which does not inspire confidence.)

You are also not using an authenticated encryption algorithm, which means that an attacker could alter the ciphertext without being detected. Depending on the plaintext and the use case, this could be used by an attacker to either surreptitiously alter, or even recover, the encrypted data. You should use an authenticated encryption algorithm like XChaCha20-Poly1305, AES-GCM-SIV or AES-GCM.

I would also recommend that you switch away from using CryptoJS (which from a quick glance looks dated and quite immature), to a more robust and modern cryptographic library like noble-ciphers.

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

No branches or pull requests

1 participant