Description

When the WooRouterV2 is used to perform a swap, the user may pay in native ETH:

function swap(
    address fromToken,
    address toToken,
    uint256 fromAmount,
    uint256 minToAmount,
    address payable to,
    address rebateTo
) external payable override nonReentrant returns (uint256 realToAmount) {
    require(fromToken != address(0), "WooRouter: !fromToken");
    require(toToken != address(0), "WooRouter: !toToken");
    require(to != address(0), "WooRouter: !to");

    bool isFromETH = fromToken == ETH_PLACEHOLDER_ADDR;
    bool isToETH = toToken == ETH_PLACEHOLDER_ADDR;
    fromToken = isFromETH ? WETH : fromToken;
    toToken = isToETH ? WETH : toToken;

    // Step 1: transfer the source tokens to WooRouter
    if (isFromETH) {
        require(fromAmount <= msg.value, "WooRouter: fromAmount_INVALID");
        IWETH(WETH).deposit{value: msg.value}();
        TransferHelper.safeTransfer(WETH, address(wooPool), fromAmount);
    } else {
        TransferHelper.safeTransferFrom(fromToken, msg.sender, address(wooPool), fromAmount);
    }
    
    // [...]

However, if the caller accidentally specifies a fromAmount that is less than, instead of being equal to, the msg.value sent with the call, that resulting WETH stays in the contract.

Impact

Any extra WETH becomes stuck in the contract. Similarly, if value is accidentally sent but fromToken is not set to ETH_PLACEHOLDER_ADDR, then the native ETH becomes stuck in the contract. In these cases, the owner needs to manually call inCaseTokenGotStuck to retrieve the value.

Recommendations

If it is intended that any extra ETH, wrapped or not, that is mistakenly left in the contract should be treated as a donation to the protocol, then it should explicitly be transferred to the pool reserves or the fee recipient. On the other hand, if the funds will be returned to the user, it would save effort and increase trust to just require that fromAmount == msg.value in the isFromEth case, and msg.value == 0 in the other branch, so that the transaction reverts if this mistake is made.

Remediation

This issue has been acknowledged by WOOFI, and a fix was implemented in commit 11183887↗.