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

6 stars 2 forks source link

Lone Peanut Swallow - Incorrect check in `WinnablesTicketManager.withdrawTokens` #622

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

Lone Peanut Swallow

Low/Info

Incorrect check in WinnablesTicketManager.withdrawTokens

Summary

The method implements an incorrect check on the amount being withdrawn, allowing only withdrawals of the contract's full token balance.

Vulnerability Detail

WinnablesTicketManager.sol#L295 implements a check with the incorrect inequality direction: by checking that amount < balance, the method disallows any call that attempts to withdraw less tokens than the contract's balance.

Impact

Low. Incorrectly implemented token withdrawal method doesn't allow for partial withdrawals.

Code Snippet

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

Tool used

Manual Review

Recommendation

     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