Description

The WooCrossChainRouterV4 contract handles cross-chain swaps over StarGate. For most of its actions, it charges a fee in the form of the token that has been offered for the cross-chain operation. To retrieve these fee tokens, someone has to call the claimFee function, which rudimentarily transfers the entire balance of the specified token to the feeAddr, a state variable that can be set by the contract owner.

function claimFee(address token) external nonReentrant {
    require(feeAddr != address(0), "WooCrossChainRouterV4: !feeAddr");
    uint256 amount = _generalBalanceOf(token, address(this));
    if (amount > 0) {
        if (token == ETH_PLACEHOLDER_ADDR) {
            TransferHelper.safeTransferETH(feeAddr, amount);
        } else {
            TransferHelper.safeTransfer(token, feeAddr, amount);
        }
    }
}

Under normal circumstances, this function is completely safe, as the feeAddr is a trusted address, and the contract is not expected to hold any tokens, as most of its operations are to be performed atomically. However, if the contract is left with some tokens, as it is the case when a cross-chain operation fails (i.e due to StarGate failure), the claimFee function can be called by anyone to retrieve these tokens.

Impact

This can be considered a griefing vector, as the contract owner might not be aware of the leftover tokens and might not be able to retrieve them in a timely manner, leading to a potential loss of funds for the user that initiated the cross-chain operation.

Note that the cross-chain router ends up holding assets if sgReceive reverts, which can happen due to a gas attack if any of the tokens supported by the swap router are compromised. In the StarGate contract, if sgReceive reverts, the call can be retried by anyone, and before the call is retried, the cross-chain router holds the funds.

Recommendations

We recommend opting for either of the two approaches to mitigate this issue:

1. Only allow the claimFee function to be called by the contract owner or a trusted party. Even though this does not completely solve the issue, it reduces the potential attack surface.

2. Implement a cumulative fee mechanism, where the fee is stored in a separate contract variable, such as a mapping, upon each cumulative operation. This way, even though the fee can still be claimed by anyone, any leftover tokens would not be accounted for in the variable, so they would not be incorrectly sent out. If tokens truly get stuck, the contract owner can still retrieve them at any time via the inCaseTokenGotStuck function.

Remediation

Additionally, the WOOFI team has stated that:

Not need to fix. The feeAddr can only be set by contract owner, which is a multi-sig safe. Even if some leftover tokens are there, the typical rescue method is also we claim the leftover tokens and send back to our user; which works well no matter whether attackers call claimFee or not.