030.md

Low Tangerine Cod

Medium

users are being overcharged more than required on BOTH_TRANSFER

Summary

Incorrect formula inside the condition if (functionToDo == FunctionToDo.BOTH_TRANSFER) leads to overcharging users.

Root Cause

Whenever user withdraw their funds and liquidation happened. There will be a call to another chain to get some funds. Sometimes this code will be triggered

        if (functionToDo == FunctionToDo.TOKEN_TRANSFER) {
            // Define options with native fee required in dst chain
            // to transfer this USDa using 'addExecutorNativeDropOption' with fee amount and this chain address where to get
            _options = OptionsBuilder.newOptions().addExecutorLzReceiveOption(500000, 0).addExecutorNativeDropOption(
-->                    uint128(feeForTokenTransfer.nativeFee),
                    bytes32(uint256(uint160(dstGlobalVariablesAddress)))
                );
     }
    ...
            } else if (functionToDo == FunctionToDo.BOTH_TRANSFER) {
            _options = OptionsBuilder.newOptions().addExecutorLzReceiveOption(700000, 0).addExecutorNativeDropOption(
                    // Since, we have 3 collaterals, we are passing native fee required for 3 OFT transfers
                    // and 1 native transfer,so we are multiplying oft fee with 3.
                    // Since feeForCollateralTransfer includes the fee with native token needed, we are subtracting the native token
                    // needed to get the fee alone
-->                    uint128(5 * feeForTokenTransfer.nativeFee + (feeForCollateralTransfer.nativeFee - collateralTokenTransferData.ethToSend)),
                    bytes32(uint256(uint160(dstGlobalVariablesAddress)))
                );
        }

Core_logic/GlobalVariables.sol#L290 In FunctionToDo.TOKEN_TRANSFER there isfeeForTokenTransfer.nativeFee, in FunctionToDo.BOTH_TRANSFER 5*feeForTokenTransfer.nativeFee. Which basically means there should be 5 .send functions triggered for oft tokens.

Lets look how many .send actually being triggered

            } else if (oappData.functionToDo == FunctionToDo.COLLATERAL_TRANSFER || oappData.functionToDo == FunctionToDo.BOTH_TRANSFER) {
            //Since we need to send ETH with OFt, we can do this by sending single transaction
            _options = OptionsBuilder.newOptions().addExecutorLzReceiveOption(280000, 0).addExecutorNativeDropOption(
                    // Pass the ETH amount needed
                    uint128(oappData.collateralTokenTransferData.ethToSend),
                    bytes32(uint256(uint160(oappData.collateralTokenTransferData.recipient)))
                );
            // Define the SendParam to send OFT
            SendParam memory _sendParam = SendParam(
                dstEid,
                bytes32(uint256(uint160(oappData.oftTransferData.recipient))),
                oappData.oftTransferData.tokensToSend,
                oappData.oftTransferData.tokensToSend,
                _options,
                "0x",
                "0x"
            );
            // Quote the native fee
            _fee = usda.quoteSend(_sendParam, false);
            // Get the funds from treasury, since the sender is global variables contract
            treasury.transferFundsToGlobal([
                    oappData.collateralTokenTransferData.ethToSend,
                    oappData.collateralTokenTransferData.weETHToSend,
                    oappData.collateralTokenTransferData.rsETHToSend,
                    oappData.oftTransferData.tokensToSend
                ]
            );
            // Send oft to src chain
-->            usda.send{value: _fee.nativeFee}(_sendParam, _fee, address(this)); // 1
            // Change the options since we dont need to send ETH
            _options = OptionsBuilder.newOptions().addExecutorLzReceiveOption(100000, 0);

            // Collateral tokens to send to src chain
            uint256[2] memory tokensToSend = [
                oappData.collateralTokenTransferData.weETHToSend,
                oappData.collateralTokenTransferData.rsETHToSend
            ];
            // Loop through the OFts to send tokens from this to src
            for (uint8 i = 0; i < tokensToSend.length; i++) {
                if (tokensToSend[i] > 0) {
                    _sendParam = SendParam(
                        dstEid,
                        bytes32(uint256(uint160(oappData.collateralTokenTransferData.recipient))),
                        tokensToSend[i],
                        tokensToSend[i],
                        _options,
                        "0x",
                        "0x"
                    );
                    _fee = usda.quoteSend(_sendParam, false);
                    address assetAddress;
                    if (i == 1) {
                        assetAddress = borrowInstance.assetAddress(IBorrowing.AssetName.rsETH);
                    } else {
                        assetAddress = borrowInstance.assetAddress(IBorrowing.AssetName(i + 2));
                    }
                    // Get the Collateal address from borrowing contract
-->                    IOFT(assetAddress).send{value: _fee.nativeFee}( _sendParam,_fee,address(this));// tokensToSend.length
                }
            }
        }

contracts/Core_logic/GlobalVariables.sol#L585

1+ tokensToSend.length = 3. which means formula is incorrect

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

users are being overcharged in BOTH_TRANSFER, which means protocol will pay for it

PoC

No response

Mitigation

        } else if (functionToDo == FunctionToDo.BOTH_TRANSFER) {
            _options = OptionsBuilder.newOptions().addExecutorLzReceiveOption(700000, 0).addExecutorNativeDropOption(
                    // Since, we have 3 collaterals, we are passing native fee required for 3 OFT transfers
                    // and 1 native transfer,so we are multiplying oft fee with 3.
                    // Since feeForCollateralTransfer includes the fee with native token needed, we are subtracting the native token
                    // needed to get the fee alone
-                    uint128(5 * feeForTokenTransfer.nativeFee + (feeForCollateralTransfer.nativeFee - collateralTokenTransferData.ethToSend)),
+                    uint128(3 * feeForTokenTransfer.nativeFee + (feeForCollateralTransfer.nativeFee - collateralTokenTransferData.ethToSend)),
                    bytes32(uint256(uint160(dstGlobalVariablesAddress)))
                );
        }