brix / crypto-js

JavaScript library of crypto standards.
Other
15.89k stars 2.39k forks source link

`WordArray.random()` does not clamp, which leads to inconsistent hashes depending on how the result is further used #395

Open blu3r4y opened 2 years ago

blu3r4y commented 2 years ago

I am not super sure if this is really a bug, but please judge by the following example:

const CryptoJS = require("crypto-js");

const msg1 = CryptoJS.lib.WordArray.random(1);
const hmac1 = CryptoJS.HmacSHA256(msg1, "key");

const msg2 = CryptoJS.enc.Hex.parse(CryptoJS.enc.Hex.stringify(msg1));
const hmac2 = CryptoJS.HmacSHA256(msg2, "key");

console.log("messages equal?", String(msg1) === String(msg2));
console.log(String(msg1));
console.log(String(msg2));

console.log();

console.log("hmac equal?", String(hmac1) === String(hmac2));
console.log(String(hmac1));
console.log(String(hmac2));

What is this code doing?

What would I expect here?

I would expect hmac1 == hmac2

What is actually happening?

While the messages are equal, the HMAC isn't. See an example output for the snippet above:

messages equal? true
2f
2f

hmac equal? false
621e65b191f600c38ffa4d49ed1473f1cd429138cdfd19d5a65ac4d5c0e60345
e901cdeed5dc3430253d155800a2e88885ac25f0b478052525c6777aca323daf

What is the root cause?

This behavior only occurs with random messages that are not multiples of 4 bytes. It can be mitigated if one does a msg1.clamp() right after creation of the random message.

I have the feeling that the library should .clamp() the result from WordArray.random() to avoid inconsistent behavior. What do you think?

juanjawbone commented 1 year ago

Perhaps this answer applies here too?

https://github.com/brix/crypto-js/issues/387#issuecomment-949925458