sherlock-audit / 2024-01-rio-vesting-escrow-judging

3 stars 2 forks source link

fibonacci - A recipient can withdraw all funds before the end of the vesting period in case of using a token with multiple addresses #46

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

fibonacci

medium

A recipient can withdraw all funds before the end of the vesting period in case of using a token with multiple addresses

Summary

The VestingEscrow::recoverERC20 function allows for the recovery of any ERC20 token to the recipient. However, this could be exploited in cases where the vesting token has multiple addresses, allowing a recipient to withdraw all funds immediately.

Vulnerability Detail

According to the contest page any, with the exception of rebasing tokens and tokens that charge a fee on transfer are supposed to be supported.

The VestingEscrow::recoverERC20 function allows for the withdrawal of tokens sent to the contract, checking that the address of the recovered token is different from the vesting token. Otherwise, you can only withdraw the excess.

function recoverERC20(address token_, uint256 amount) external {
    uint256 recoverable = amount;
    if (token_ == address(token())) {
        uint256 available = token().balanceOf(address(this)) - (locked() + unclaimed());
        recoverable = Math.min(recoverable, available);
    }
    if (recoverable > 0) {
        IERC20(token_).safeTransfer(recipient(), recoverable);
        emit ERC20Recovered(token_, recoverable);
    }
}

However, some ERC20 tokens have multiple valid contract addresses that serve as entrypoints for manipulating the same underlying storage. In this case, the recipient can bypass the check and withdraw all funds simply by using a different token address.

Impact

A recipient can withdraw all funds ahead of time.

Code Snippet

https://github.com/sherlock-audit/2024-01-rio-vesting-escrow/blob/main/rio-vesting-escrow/src/VestingEscrow.sol#L202-L212

Tool used

Manual Review

Recommendation

Compare the balance of the vesting token before and after recovering.

Duplicate of #62

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

_rahul commented:

Invalid: Non standard / weird-tokens are not considered valid by default unless these tokens are explicitly mentioned in the README.