Bald Sky Alligator - Admins can only withdraw LINK or ERC20 tokens from WinnablesTicketManager contract with the exact amount of the tokens belong to this contract

[sherlock-admin4]

Bald Sky Alligator

Low/Info

Admins can only withdraw LINK or ERC20 tokens from WinnablesTicketManager contract with the exact amount of the tokens belong to this contract

Summary

The WinnablesTicketManager::withdrawTokens method only allows admin to withdraw the exact amount of tokens belong to this contract.

So that admins cannot withdraw less than the balance of the tokens belong to WinnablesTicketManager contract.

Root Cause

In WinnablesTicketManager.sol:295 the check for the amount of tokens to be withdraw from this contract was not implemented correctly.

https://github.com/sherlock-audit/2024-08-winnables-raffles/blob/main/public-contracts/contracts/WinnablesTicketManager.sol#L295

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

If the admins want to successfully call the WinnablesTicketManager::withdrawTokens method, they have to input the amount parameter equal to balance of the withdrawn token belong to WinnablesTicketManager contract.

So there could be a scenario that the admin only want to withdraw X amount of LINK tokens from WinnablesTicketManager for paying Chainlink services used by other contract. But now, he has to withdraw all the LINK in WinnablesTicketManager contract and then deposit again the amount that he actually one. This's a bit inconvenient and create unnecessary transactions.

PoC

Here is the test case that failed to withdraw tokens from WinnablesTicketManager contract

it('Should not be able to withdraw less than the balance of LINK', async () => {
  const balance = await link.balanceOf(manager.address);
  const withdrawAmount = balance.sub(1);
  await expect(manager.connect(signers[0]).withdrawTokens(link.address, withdrawAmount)).to.be.revertedWithCustomError(
    manager,
    'InsufficientBalance'
  );
});

Mitigation

Update the check in WinnablesTicketManager::withdrawTokens method as below:

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

[sherlock-admin2]

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