OpenZeppelin / cairo-contracts

OpenZeppelin Contracts written in Cairo for Starknet, a decentralized ZK Rollup
https://docs.openzeppelin.com/contracts-cairo
MIT License
797 stars 322 forks source link

[TOB] TOB-ACCOUNT-1: Signature verification allows for malleability #889

Closed technovision99 closed 4 months ago

technovision99 commented 5 months ago

Signature verification allows for malleability

Severity:Informational Difficulty: High Type: Data Validation Target: account/utils/signature.cairo

Description

When verifying a STARK/ETH signature, the corelib functions check_ecdsa_signature and recover_public_key are used. However both of these functions do not check the s value of the provided signature. As a result both (r,s) and (r,-s) are valid signatures for the same public key. Currently this is not exploitable because the signature validation is performed over the transaction hash, which is unique per transaction.

https://github.com/OpenZeppelin/cairo-contracts/blob/ef4128c61c6104852a1172e0e43e22fce1d33075/src/account/utils/signature.cairo#L11-L43

Recommendations

Short term, validate that the s value is positive, i.e. it is less than CURVE_ORDER / 2.

Long term, improve unit testing coverage to uncover edge cases like this and ensure intended behavior throughout the system.