search

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

0 stars 0 forks source link

Lack of address input validation will lock tokens in contract #38

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Functions timeLockERC721() and timeLockERC20() are used by the vault owner to timelock tokens in the vault with a specified recipient address as the only one with the right to withdraw after timelock expiry.

If a zero/incorrect recipient address is used here accidentally, the ERC20/ERC721 tokens will be locked in contract forever because no one else can withdraw them.

Proof of Concept

PoC-1: Alice, the vault owner, intending to specify Bob’s address as the timelock recipient specifies Eve’s address. Given that there is no way to cancel/correct/override this accident, Eve ends up gaining those locked tokens after timelock expiry.

PoC-2: Alice, the vault owner, intending to specify Bob’s address as the timelock recipient makes a typo in Bob’s address to specify an address for which she does not have a private key. Given that there is no way to cancel/correct/override this accident, those tokens after timelock expiry end up locked in vault forever.

PoC-3: Alice, the vault owner, intending to specify Bob’s address as the timelock recipient makes a mistake in transaction specification to specify a zero address (for which no one has a private key). Given that there is no way to cancel/correct/override this accident, those tokens after timelock expiry end up locked in vault forever.

https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L524-L554

https://github.com/code-423n4/2021-05-visorfinance/blob/e0f15162a017130aa66910d46c70ee074b64dd40/contracts/contracts/visor/Visor.sol#L578-L612

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider adding a correction window timer of say 1 block within which the vault owner is allowed to call timeLockERC721()/timeLockERC20() once more with a different recipient in case they specified an incorrect address the first time.

ghost commented 3 years ago

sponseor disputed disagree with severity 0 client should not call with zero/incorrect recipient address

ghoul-sol commented 3 years ago

This is equivalent of burning. Agree with sponsor.