sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

0xKartikgiri00 - Lack of Zero Amount Check in `Stablecoin::erc20Rescue`function, leads the loss of user funds. #51

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

0xKartikgiri00

medium

Lack of Zero Amount Check in Stablecoin::erc20Rescuefunction, leads the loss of user funds.

Summary

The function erc20Rescue in the Stablecoin contract does not check for a zero amount before transferring tokens to the destination address. Even though erc20Rescue function can be only call by the SUPPORT_ROLE but cause of the mistake or confusion the trusted role can transfer 0 amount to destination address. Which will leads to the loss of user funds. So that's why it is important to explicitly restrict the transfer of 0 amount.

Vulnerability Detail

The erc20Rescue function in the Stablecoin contract allows only SUPPORT_ROLE to transfer tokens that are accidentally sent to the contract back to a specified destination address. But, it does not include a check to ensure that the amount being transferred is greater than zero. As a result, if a user attempts to rescue amount 0 , then the SUPPORT_ROLE will call the erc20Rescue function to transfer 0 amount which doesn't make any sense. or the SUPPORT_ROLE can send the 0 amount to the genuine destination address which will leads to the loss of funds for user.

Impact

If a user accidentally sends tokens to the Stablecoin contract and by mistake user attempts to rescue 0 amount or SUPPORT_ROLE pass 0 amount by mistake to erc20Rescue function, then destination contract will receive 0 tokens instead of the intended amount.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/stablecoin/Stablecoin.sol#L138

Tool used

Manual Review

Recommendation

The recommended mitigation is to ensure that the erc20Rescue function checks that the amount being transferred is greater than zero before executing the transfer.

Mitigation:=

 function erc20Rescue(ERC20PermitUpgradeable token, address destination, uint256 amount)
        external
        onlyRole(SUPPORT_ROLE)
    {
+       require(amount > 0, "Stablecoin: amount should be greater than 0");
        token.safeTransfer(destination, amount);
    }
sherlock-admin3 commented 8 months ago

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

WangAudit commented:

how can users lose funds when you transfer to them 0 tokens…

kartik-giri commented 8 months ago

The user never get the amount back!!