Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sherlock-admin2 opened this issue Nov 13, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Nov 13, 2024

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

@sherlock-admin3 sherlock-admin3 changed the title Handsome Sandstone Cottonmouth - Support role may fail to rescue tokens due to unchecked return values CipherHawk - Support role may fail to rescue tokens due to unchecked return values Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant