Skip to content

Latest commit

 

History

History
79 lines (58 loc) · 3.09 KB

082.md

File metadata and controls

79 lines (58 loc) · 3.09 KB
Raw

Bright Violet Crocodile

Medium

Fee change from fee on transfer tokens will severely harm the protocol

Summary

Fee on transfer tokens may change their fee at any time which will mess up completely the accounting of all functionality in AmirX and StablecoinHandler.

Root Cause

In both AmirX and StablecoinHandler, fee on transfers are dealt with by simply changing the ratio from ss.oAmount to ss.tAmount, that is, instead of swapping for example 1000 tokens A for 1000 B, they know tokens A have a 1% fee on transfer so they swap 1010 tokens A for 1000 tokens B. However, the fee may change at any time and become 2% or 0.5%, which will mess up the accounting and make the protocol take a loss or the user.

Internal pre-conditions

None.

External pre-conditions

Fee on transfer tokens change their fee and frontrun stablecoin swap calls.

Attack Path

  1. Fee on transfer tokens change their fee.
  2. Any function that does a _stablecoinSwap() in AmirX or StablecoinHandler is called.

The simplest example is calling StablecoinHandler::stablecoinSwap() with an amount of USDC of 1010.10 to receive 1000 eUSDC (assuming USDC has a 1% fee although it does not currently). If the fee changes to 2% frontrunning this call, the 1010.10 USDC will only send to the liquiditySafe an amount of 1010.10 * 98 / 100 = 989.9 instead of 1000 USDC, taking the protocol the loss.

Impact

The protocol takes losses whenever a token changes the value of the fee on transfer or the user is penalized if the fee decreases instead.

PoC

function _stablecoinSwap(
    address wallet,
    StablecoinSwap memory ss
) internal {
    if (
        ss.stablecoinFeeCurrency != address(0) &&
        ss.stablecoinFeeSafe != address(0)
    )
        ERC20PermitUpgradeable(ss.stablecoinFeeCurrency).safeTransferFrom(
            wallet,
            ss.stablecoinFeeSafe,
            ss.feeAmount
        );

    // Handle the transfer or burning of the origin currency:
    // If the origin is a recognized stablecoin (XYZ), burn the specified amount from the wallet.
    if (isXYZ(ss.origin)) {
        Stablecoin(ss.origin).burnFrom(wallet, ss.oAmount);
    } else {
        ERC20PermitUpgradeable(ss.origin).safeTransferFrom(
            wallet,
            ss.liquiditySafe,
            ss.oAmount 
        );
    }

    // Handle the minting or transferring of the target currency:
    // If the target is a recognized stablecoin (XYZ), mint the required amount to the destination address.
    if (isXYZ(ss.target)) {
        Stablecoin(ss.target).mintTo(ss.destination, ss.tAmount);
    } else {
        ERC20PermitUpgradeable(ss.target).safeTransferFrom( //@audit stuck if target becomes xyz
            ss.liquiditySafe,
            ss.destination,
            ss.tAmount 
        );
    }
}

Mitigation

Add checks to StablecoinHandler::_stablecoinSwap() to ensure the fees applied in the transfers match the expected values used when calculating ss.oAmount and ss.tAmount.