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

0 stars 0 forks source link

Lack of non-zero check in function `timeLockERC20` and `timeLockERC721` could cause funds being locked #75

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

shw

Vulnerability details

Impact

The functions timeLockERC20 and timeLockERC721 lack a non-zero address check on the parameter recipient. If this parameter is set to address(0) by accident, funds are locked in the vault, and even the owner could not withdraw them.

Proof of Concept

If the recipient is provided as address(0), no one could simply call the functions timeUnlockERC20 and timeUnlockERC721 to withdraw the fund since the requirement of the msg.sender being the recipient. Besides, in functions transferERC20 and delegatedTransferERC20, the require statements (at lines 419 and 456) ensure the vault's balance to be greater than timelockERC20Balances[token] after the withdraw. Thus, the locked funds are not withdrawable.

Referenced code: Visor.sol#L419 Visor.sol#L456 Visor.sol#L583-L612 Visor.sol#L529-L554 Visor.sol#L632 Visor.sol#L569

Tools Used

None

Recommended Mitigation Steps

Add require(recipient != address(0)) in both functions.

ghost commented 3 years ago

sponsor acknowledge dispute severity 0 0 address should not be used as param. It is not used in our platform nevertheless we will remove ability in next version

ghoul-sol commented 3 years ago

Duplicate of #38, marking as invalid