fix CLSAG verification. #552
Conversation
We were not setting c1 to the last calculated c during verification, instead keeping it set to the one provided in the signature.
kayabaNerve
added
bug
|
No. This is a critical in not only monero-serai yet in the multisig functionality specifically. This would cause invalid participants in the FROST process to not be flagged as it'd think the signature is still (wrongly) valid. On the one hand, monero-serai wasn't audited (queued), I haven't done my final pass, and it's not a 1.0. On the other, I'm fucking pissed this made it through and horrified. At best, I can be proud monero-serai did enough right it caught enough other people this was itself caught. Matrix is acting up and I don't have any messages from you. Please reach out on Discord (Serai Discord to find me) or Telegram (kayabaNerve), or let me know if I need to make a new Matrix account on monero.social so we can sync on this. |
|
All versions of monero-serai are currently yanked for an independent reason (several months ago). I never re-published after (I don't even think I've published the audited bitcoin-serai), so no yanking needed due to this. Still outrageous. |
|
Expansion of basic tests, as even those would've caught this (further emphasizing the fault here), fuzz tests, and prioritizing #549? That's my immediate thoughts on the matter. Possibly addition of code coverage evaluators? That'd highlight such missing cases... |
|
Also, to be perfectly clear, only myself is to blame here. All my frustration is self-directed. |
|
I don't use any of those other platforms. If the Matrix issues are causing problems in other chats, it might be best to make a monero.social account otherwise I could make a matrix.org account. |
|
+1 for more tests |
|
I ended up making a matrix.org account: boog900:matrix.org |
We were not setting c1 to the last calculated c during verification, instead keeping it set to the one provided in the signature, allowing forgeries.
@kayabaNerve I did message you on matrix but it seems there are still issues so I decided just to open this PR, although this is bad AFAIK no one relies on this ATM.