code423n4 commented 12 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/Well.sol#L113-L132 https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/Well.sol#L195-L204

Vulnerability details

Impact

The ecrecover function is an inherent cryptographic function within Solidity which enables the retrieval of the signer's address messages which have been signed with their private key. Of course, this is very important when it comes to verifying the authenticity of a message from a cryptographic standpoint. The team did not implement ECDSA correctly within Well.sol pertaining to the permit and delegateBySig functions, because the values of s and v are not sanitised. The curve is fundamentally symmetrical on the x-axis and therefore permits signatures to be replayed with the same x value (in this instance r) but a different y value (in this instance s).

Proof of Concept

Imagine userA signs a transaction with their public key. The transaction is included within the blockchain and a miner eventually picks it up. Among all of the transactions that are awaiting to be successful, there is userB whom is malicious, and has the intention of executing transaction malleability against userA or, for this matter, any user who is interacting with the protocol. userB will retrieve the values of v, r, and s for userA and replay the transaction with the same signature by simply tweaking the s value and maintaining the same r value which was used for userA.

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/Well.sol#L113-L132

https://github.com/code-423n4/2023-07-moonwell/blob/main/src/core/Governance/Well.sol#L195-L204

Tools Used

Manual review

Recommended Mitigation Steps

Ideally, v has to be checked so that it is equal to 27 or 28, and s has to be ensured that it is within the lower half order of the elliptic curve, so that it is less than 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A1.

You can also utilise OpenZeppelin's default ECDSA library: https://github.com/OpenZeppelin/openzeppelin-sdk/blob/master/packages/lib/contracts/cryptography/ECDSA.sol

I would also like to add into reference the exact same bug present on a project named Platypus Finance Governance Staking (dated 9 Apr 2022) which was audited by Omniscia: https://omniscia.io/platypus-finance-governance-staking/manual-review/Ptp-PTP

Further references: