sherlock-audit / 2024-11-telcoin-judging

0 stars 0 forks source link

CipherHawk - Support role may fail to rescue tokens due to unchecked return values #228

Open sherlock-admin2 opened 1 week ago

sherlock-admin2 commented 1 week ago

CipherHawk

High

Support role may fail to rescue tokens due to unchecked return values

Summary

The erc20Rescue function in Stablecoin.sol uses SafeERC20 for token transfers but does not handle scenarios where the safeTransfer might fail silently. This will cause rescue operations to fail without reverting, leading to tokens being stuck in the contract.

Root Cause

In Stablecoin.sol at lines XX-YY, the erc20Rescue function calls token.safeTransfer(destination, amount) without ensuring that the transfer was successful, relying solely on SafeERC20 to handle potential failures.

Internal pre-conditions

  1. The contract must hold a balance of the specified ERC20 token.

  2. The SUPPORT_ROLE holder initiates the rescue operation.

External pre-conditions

External pre-conditions:

  1. The ERC20 token may have non-standard implementations that do not return boolean values on transfers.

  2. The destination address must be a valid recipient capable of receiving the tokens.

Attack Path

Attack Path:

  1. The SUPPORT_ROLE holder calls erc20Rescue to transfer tokens to a destination.

  2. If the ERC20 token's transfer function fails or does not return a success value, SafeERC20 may not revert the transaction.

  3. The tokens remain locked in the contract as the transfer did not execute successfully.

Impact: Tokens intended to be rescued remain inaccessible within the contract, leading to potential loss of funds and user dissatisfaction.

Mitigation:

Ensure Revert on Failure: Modify the erc20Rescue function to include explicit require statements that confirm the success of the safeTransfer operation.

function erc20Rescue( IERC20 token, address destination, uint256 amount ) external onlyRole(SUPPORT_ROLE) { token.safeTransfer(destination, amount); require(token.balanceOf(destination) >= amount, "Transfer failed"); }

Leverage SafeERC20 Properly: Continue using SafeERC20 to handle various ERC20 implementations, ensuring that all potential edge cases are accounted for.

Implement Event Emissions: Emit events upon successful transfers to facilitate monitoring and verification of rescue operations.

Impact

No response

PoC

No response

Mitigation

No response