code-423n4 / 2021-05-visorfinance-findings

0 stars 0 forks source link

lock/unlock signatures may be replayed on a different contract/chain #41

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

lock() and unlock() functions can be called by delegate with signed permission from owner. The permission signature includes a nonce which prevents replay attacks over time. However, it neither includes this contract’s address nor the chain ID.

The impact is that if this contract is ever redeployed on the same mainnet at a different address for some reason or deployed on a Layer 2 (Optimism, Arbitrum, Polygon etc.) or any other EVM-compatible chain, the previously used signature may be replayed on the new chain leading to unexpected outcomes.

Proof of Concept

https://github.com/code-423n4/2021-05-visorfinance/blob/dda9be0bcbf4b2c549608405ccdf9469bc2cbaf1/contracts/contracts/visor/Visor.sol#L277

https://github.com/code-423n4/2021-05-visorfinance/blob/dda9be0bcbf4b2c549608405ccdf9469bc2cbaf1/contracts/contracts/visor/Visor.sol#L327

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add address(this) and chain ID to the signed permission.

ghost commented 3 years ago

sponsor disputed implemented by EIP712.sol _buildDomainSeparator function

ghoul-sol commented 3 years ago

Sponsor is right, marking as invalid