stake.link

stake.link
DeFiHardhatBridge
27,500 USDC
View results
Submission Details
Severity: high
Invalid

Reentrancy Risk Due to Unchecked Ether Transfer in WrappedTokenBridge Contract

Summary

The WrappedTokenBridge contract may be vulnerable to reentrancy attacks due to the use of a low-level .call for refunding excess Ether to the _sender.

Vulnerability Details

A user or contract initiates a token transfer to the WrappedTokenBridge contract, which triggers the onTokenTransfer method due to the ERC677 transferAndCall functionality.

onTokenTransfer Execution: The onTokenTransfer method is an external function that can be called by anyone who transfers the appropriate tokens to the contract. It processes the incoming tokens and prepares to wrap them for cross-chain transfer.

Internal Call to _transferTokens: Within onTokenTransfer, the internal _transferTokens method is called to handle the actual wrapping of tokens and manage the transfer logistics, including refunding any excess Ether.

The reentrancy concern arises at the point where _transferTokens sends Ether back to the _sender. If _sender is a contract, it could potentially execute code (fallback or receive function) that could re-enter the WrappedTokenBridge contract .

In the _transferTokens method, line 153 is responsible for refunding excess Ether to the _sender:

(bool success, ) = _sender.call{value: msg.value - fees}("");
if (!success) revert TransferFailed();

The use of .call with an empty data payload is a flexible way to send Ether and is recommended over transfer and send post-EIP-1884 due to gas stipend limitations. However, .call forwards all remaining gas by default, which can allow the recipient to perform actions that could lead to reentrancy attacks.

Impact

The potential reentrancy vulnerability could allow a malicious contract to execute the fallback function and interact with the WrappedTokenBridge contract while the refund transaction is still in progress. This could lead to state inconsistencies or other unintended effects since onTokenTransfer() is not protected against reentrancy.

##Proof of concept working on code.: ran slither...
A contract (malicious or otherwise) calls the onTokenTransfer function, providing tokens and additional Ether.

Refund Process: The _transferTokens function calculates the necessary fees and attempts to refund the excess Ether to the contract that initiated the transfer.

Fallback Function: The calling contract's fallback function is triggered by the receipt of Ether and makes a reentrant call back into the WrappedTokenBridge contract, potentially targeting functions that could be exploited.

Exploitation: If the WrappedTokenBridge contract has any public or external functions that change state and are not protected by a reentrancy guard, the malicious contract could take advantage of the reentrant call to manipulate the contract's state or drain funds.

Tools Used

Recommendations

Implement a reentrancy guard (e.g., using OpenZeppelin's ReentrancyGuard contract) on all public and external functions that perform state changes and Ether transfers

Change the refund mechanism to a pull pattern, where users withdraw funds themselves, reducing the risk of reentrancy and DoS attacks.
Reentrancy . Also add proper checks and reentrancy guard to mechanism. lol

`
contract WrappedTokenBridge is Ownable, CCIPReceiver {

// Mapping to track refunds owed to addresses
mapping(address => uint256) private refunds;



/**
 * @notice Internal function to wrap and transfer tokens to a destination chain
 * @param _destinationChainSelector id of destination chain
 * @param _sender address of token sender
 * @param _receiver address to receive tokens on destination chain
 * @param _amount amount of tokens to transfer
 * @param _payNative whether fee should be paid natively or with LINK
 * @param _maxLINKFee call will revert if LINK fee exceeds this value
 **/
function _transferTokens(
    uint64 _destinationChainSelector,
    address _sender,
    address _receiver,
    uint256 _amount,
    bool _payNative,
    uint256 _maxLINKFee
) internal returns (bytes32 messageId) {
    // ... existing wrapping logic ...

    // Calculate fees and prepare for cross-chain transfer
    // ...

    if (_payNative) {
        // Calculate the fees required for the transfer
        uint256 fees = calculateFees(_destinationChainSelector, _amount, _payNative);
        
        // Instead of sending Ether, update the refund balance
        if (msg.value > fees) {
            refunds[_sender] += msg.value - fees;
        }

        // Proceed with the cross-chain transfer logic
        // ...
    } else {
        // Handle the LINK fee payment logic
        // ...
    }

    // Emit an event for the transfer
    // ...

    return messageId;
}

// ... other functions ...

/**
 * @notice Allows users to withdraw their refunds
 **/
function withdrawRefund() external {
    uint256 refundAmount = refunds[msg.sender];
    require(refundAmount > 0, "No refund available");

    // Set the refund amount to zero before sending to prevent reentrancy
    refunds[msg.sender] = 0;

    // Send the refund to the caller
    (bool success, ) = msg.sender.call{value: refundAmount}("");
    require(success, "Refund transfer failed");
}

}

`

Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.