sherlock-audit / 2024-08-winnables-raffles-judging

6 stars 2 forks source link

Daring Parchment Goblin - Admin could not withdraw tokens below contract balance in `WinnablesTicketManager` contract #633

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

Daring Parchment Goblin

Low/Info

Admin could not withdraw tokens below contract balance in WinnablesTicketManager contract

Vulnerability Detail

The function withdrawTokens in the WinnablesTicketManager contract allows the admin to withdraw LINK or any ERC20 tokens. Only the admin can call this function. However, the function checks if the amount to withdraw is less than the balance, meaning that the admin can only withdraw the full balance of the token.

    function withdrawTokens(address tokenAddress, uint256 amount) external onlyRole(0) {
        IERC20 token = IERC20(tokenAddress);
        uint256 balance = token.balanceOf(address(this));
@>      if (amount < balance) revert InsufficientBalance();
        token.safeTransfer(msg.sender, amount);
    }

From the name of the revert message, which is InsufficientBalance, it seems that it is intended to be able to withdraw amount of tokens which is less than the balance.

Impact

Low impact, the function can be updated to allow the admin to withdraw any amount of tokens, not only the full balance.

Code Snippet

https://github.com/Winnables/public-contracts/blob/9474451539b7081f5b2e246c68b90a16e7c55b31/contracts/WinnablesTicketManager.sol#L295

Tool used

Manual Review

Recommendation

Update the function to check if the amount is greater than the balance:

function withdrawTokens(address tokenAddress, uint256 amount) external onlyRole(0) {
        IERC20 token = IERC20(tokenAddress);
        uint256 balance = token.balanceOf(address(this));
-       if (amount < balance) revert InsufficientBalance();
+       if (amount > balance) revert InsufficientBalance();
        token.safeTransfer(msg.sender, amount);
    }
sherlock-admin2 commented 2 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/Winnables/public-contracts/pull/6