search

monero-project / monero

Monero: the secure, private, untraceable cryptocurrency
https://getmonero.org
Other
9k stars 3.12k forks source link

Copy and paste error in ge_fromfe_frombytes_vartime() (crypto-ops.c) leads to invalid result #3763

Open ogrebgr opened 6 years ago

ogrebgr commented 6 years ago

In src/crypto/crypto-ops.c the function ge_fromfe_frombytes_vartime() probably uses old version of the code from fe_frombytes.c

On line 2225 in crypto-ops.c there is: int64_t h9 = load_3(s + 29) << 2;

In fe_frombytes.c same line is: crypto_int64 h9 = (load_3(s + 29) & 8388607) << 2;

End result is that the ge_fromfe_frombytes_vartime() gives different (wrong) result, which leads to wrong result from hash_to_ec(), generate_key_image(), etc.

Implementations in other languages use the approach from fe_frombytes.c.

vtnerd commented 6 years ago

This was probably intentional - the same commit has both versions. The int ge_frombytes_vartime function is doing what the other languages are doing (unpacking a compressed y-coordinate + x-signbit). The function you referenced is mapping an arbitrary field element to some ed25519 group element. The former has to be masked because the sign bit is not part of the y coordinate. Unless there is a known security issue by doing it this way, I don't see a reason to change it.

ogrebgr commented 6 years ago

I hope you are right. It would be best if whoever did the commit confirms that it is intentional.