search

ing-bank / threshold-signatures

Threshold Signature Scheme for ECDSA
MIT License
199 stars 41 forks source link

Missing checks in ECDSA verification #4

Closed veorq closed 3 years ago

veorq commented 3 years ago

This ECDSA verification code does not do any of the sanity checks required (see e.g. https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm#Signature_verification_algorithm):

https://github.com/ing-bank/threshold-signatures/blob/8e255a5e65a7bba5dd59ef76492c7fc5f838ba3c/src/ecdsa/mod.rs#L271-L286

Probably not a big risk for the lib though.

RustMania commented 3 years ago

It is obvious that values s and r must be checked whether they are elements of Z_q. However, note that both values are of type curv::Secp256k1Scalar, where constructing a value from BigInt source applies mod q operation: https://github.com/ZenGo-X/curv/blob/master/src/elliptic/curves/secp256_k1.rs#L120

veorq commented 3 years ago

Yeah these are field elements. Was thinking of the pubkey validation and point identity check during verification.