Skip to content

Latest commit

 

History

History
85 lines (61 loc) · 3.45 KB

016.md

File metadata and controls

85 lines (61 loc) · 3.45 KB
Raw

Digital Mandarin Gecko

High

Flaw in defiToStablecoinSwap logic can cause zero-amount swaps and unintended Behavior

Summary

In the AmirX contract, there is a potential issue in the defiToStablecoinSwap function where the logic for setting the output amount (ss.oAmount) after performing a DeFi swap does not validate if the balance change is zero. This can lead to unintended behavior in downstream operations, such as conducting a stablecoin swap with an incorrect or zero amount.

Root Cause

The defiToStablecoinSwap function calculates the difference between fBalance and iBalance after a DeFi swap to set ss.oAmount, but it does not check if this difference is zero. This lack of validation can result in a stablecoin swap being executed with an incorrect or zero oAmount, which may cause inconsistent behavior compared to the swap function.

In defiToStablecoinSwap function, https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/swap/AmirX.sol#L125

In swap function, https://github.com/sherlock-audit/2024-11-telcoin/blob/main/telcoin-audit/contracts/swap/AmirX.sol#L93

swap function can do functionalities like defiToStablecoinSwap, stablecoinToDefiSwap, defiSwap by changing paramters.

Internal pre-conditions

  • The defiToStablecoinSwap function is called.
  • A DeFi swap is successfully executed, modifying the token balance.

External pre-conditions

  • The contract is not paused.
  • The caller has the SWAPPER_ROLE.
  • For defiToStablecoinSwap , input parameter ss.oAmount is not ZERO

Attack Path

  1. A user or malicious actor triggers the defiToStablecoinSwap function.
  2. The DeFi swap does not result in any balance change (i.e., fBalance - iBalance is zero).
  3. The function sets ss.oAmount to zero and proceeds with the _stablecoinSwap operation.
  4. In _stablecoinSwap, if ss.oAmount is zero, the function might still attempt to burn or transfer zero tokens: 
    if (isXYZ(ss.origin)) {
    Stablecoin(ss.origin).burnFrom(wallet, ss.oAmount);
} else {
    ERC20PermitUpgradeable(ss.origin).safeTransferFrom(
        wallet,
        ss.liquiditySafe,
        ss.oAmount
    );
}
    This can result in unintended behavior, triggering unexpected contract operations.

Impact

  • The ss.oAmount being zero can lead to:
    • Users can get target stable coin without burning.
    • Potential issues when zero-amount burns or transfers are attempted.
    • Users potentially expecting a swap result but receiving none, causing confusion and inefficiency.

PoC

uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
_defiSwap(wallet, defi); // DeFi swap that does not change balance
uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);
assert(fBalance - iBalance == 0); // Condition met
_stablecoinSwap(wallet, ss); // Executes with zero `ss.oAmount`

Mitigation

Add a condition to check if fBalance - iBalance is non-zero before assigning it to ss.oAmount and calling _stablecoinSwap:

uint256 iBalance = ERC20(ss.origin).balanceOf(wallet);
_defiSwap(wallet, defi);
uint256 fBalance = ERC20(ss.origin).balanceOf(wallet);

if (fBalance - iBalance != 0) {
    ss.oAmount = fBalance - iBalance;
}
_stablecoinSwap(wallet, ss);

This ensures that the stablecoin swap only proceeds if there is a valid output amount, preventing zero-amount burns or transfers and aligning the behavior with that of the swap function.