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

3 stars 2 forks source link

kgothatso - `VestingEscrow :: claim ` owner can front run the contract and cause a DOS attack and steal all funds #99

Closed sherlock-admin2 closed 9 months ago

sherlock-admin2 commented 9 months ago

kgothatso

high

VestingEscrow :: claim owner can front run the contract and cause a DOS attack and steal all funds

Summary

owner can front run the contract and cause a DOS attack funds can be lost before funds are claimed

Vulnerability Detail

when a user claims for funds it will revert because the owner recovered all the Ether

Impact

loss of funds

Code Snippet

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

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

         function recoverERC20(address token_, uint256 amount) external {
        if (amount > 0) {
            IERC20(token_).safeTransfer(owner(), amount);
            emit ERC20Recovered(token_, amount);
        }
    }

    /// @notice Recover any ETH to the owner.
    function recoverEther() external {
        uint256 amount = address(this).balance;
        if (amount > 0) {
            payable(owner()).sendValue(amount);
            emit ETHRecovered(amount);
        }
    }

Tool used

Manual Review

Recommendation

owner should not be able to liquidate the contract without a 51% majority to approve the transaction

nevillehuang commented 9 months ago

Invalid, admins are trusted entities trusted to not be malicious. See point 5. of sherlock rules. They retain the right to revoke tokens at any time to not allow user to claim tokens.

sherlock-admin2 commented 9 months ago

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

pratraut commented:

'invalid as issues are not explained properly and recommendation is irrelevant'