sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

zzykxx - `SUPPORT_ROLE` can transfer out `TELCOIN` tokens that are not yet distributed #210

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

zzykxx

medium

SUPPORT_ROLE can transfer out TELCOIN tokens that are not yet distributed

Summary

The SUPPORT_ROLE can withdraw TELCOIN tokens that should be distributed to council members from the CouncilMember.sol contract.

Vulnerability Detail

The SUPPORT_ROLE can call functions to recover ERC20 tokens stuck in the CouncilMember.sol contract. The CouncilMember.sol contract temporarily holds TELCOIN tokens that should be distributed to the council members.

In the erc20Rescue() function there is no check to ensure that after the rescue there's enough TELCOIN left in the contract for the council members to claim, in fact it's possible to withdraw all of them.

Impact

The CouncilMember.sol might enter a state where it's insolvent towards council members, which might not be able to withdraw their TELCOIN tokens.

Code Snippet

Tool used

Manual Review

Recommendation

When rescuing TELCOIN tokens in the erc20Rescue() function subtract the amount of tokens currently owed to council members:

function erc20Rescue(IERC20 token,address destination,uint256 amount) external onlyRole(SUPPORT_ROLE) { 
    uint256 toDistribute = 0;
    if(token == TELCOIN) {
        for (uint i = 0; i < balances.length; i++) {
            toDistribute += balances[i];
        }
    }
    token.safeTransfer(destination, amount - toDistribute);
}
sherlock-admin2 commented 8 months ago

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

takarez commented:

invalid because { thats an admin role which is considered trusted}

nevillehuang commented 8 months ago

Invalid, CouncilMember is not expected to hold any funds. The erc20Rescue is simply to retrieve any accidental donations of any erc20 tokens.