search

Chia-Network / bls-signatures

BLS signatures in C++, using the blst library for BLS12-381
Apache License 2.0
297 stars 212 forks source link

Check for same messages probably not correct? #22

Closed codablock closed 6 years ago

codablock commented 6 years ago

https://github.com/Chia-Network/bls-signatures/blob/f1570dfeb7e5c9741fd8f42ced93ec811e6944ae/src/bls.cpp#L555

Is this supposed to catch non-distinct messages? If yes, it'll only catch these if they are direct neighbors in mappedHashes. I assume you're relying on the messages being sorted here, but that's something that IMHO could go wrong easily if VerifyNative is called in a different way at any time in the future.

I'd suggest to either add a check for correct sorting in the same loop (g2_cmp(...) != CMP_GT instead of g2_cmp(...) == CMP_EQ) and add a proper comment so that the requirement is clear or add a real duplicate check (e.g., based on std::set)

mariano54 commented 6 years ago

Yeah, the purpose of this check was to prevent rogue public key attack (two identical messages). And it was supposed to be called with sorted messages.

However, now it's just called by Verify, and message are guaranteed to be distinct. I think we should just remove this check, keep this method private, and add a comment.

mariano54 commented 6 years ago

Will add to the latest PR.