sherlock-audit / 2024-02-jala-swap-judging

6 stars 4 forks source link

Anubis - Insufficient Handling of Fee-on-Transfer Tokens in _swapSupportingFeeOnTransferTokens Function #20

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

Anubis

high

Insufficient Handling of Fee-on-Transfer Tokens in _swapSupportingFeeOnTransferTokens Function

Summary

The _swapSupportingFeeOnTransferTokens function in the smart contract code contains a critical flaw due to its failure to properly handle fee-on-transfer tokens during token swaps. The function assumes that the full input token amount is used in the swap, disregarding the potential deduction of transfer fees. This leads to incorrect calculation of the output token amount, resulting in an imbalance in token pair reserves and the possibility of users receiving less than the expected amount of output tokens.

Vulnerability Detail

The root cause of the vulnerability in the code lies in the insufficient handling of fee-on-transfer tokens in the _swapSupportingFeeOnTransferTokens function.

In line 365, the code calculates the amountInput by subtracting the reserveInput from the balance of the input token in the pair. However, this calculation does not take into account the scenario where the input token is a fee-on-transfer token.

Fee-on-transfer tokens implement a fee mechanism where a percentage of the token transfer amount is deducted as a fee, which reduces the actual balance received by the recipient. In this case, the balance of the input token in the pair does not accurately reflect the amount that can be swapped, as it includes the deducted fee amount.

Therefore, when calculating the amountInput in line 365, the code should consider the possibility of fee-on-transfer tokens and adjust the calculation accordingly to handle the deducted fee amount. Failing to do so can result in incorrect swap amounts and potential loss of funds.

To exploit this vulnerability, an attacker could create a fee-on-transfer token and add it to the path of tokens to be swapped in the _swapSupportingFeeOnTransferTokens function. Since the balance of the pair does not accurately reflect the available tokens due to the fee-on-transfer mechanism, the calculation of amountInput will be incorrect. This could result in the attacker receiving more tokens than they should in the swap.

Proof of Concept (PoC) code:

1.Create a fee-on-transfer token contract:

// FeeOnTransferToken.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract FeeOnTransferToken is ERC20 {
    constructor() ERC20("FeeOnTransferToken", "FOTT") {
        _mint(msg.sender, 1000000 * 10 ** 18);
    }

    function transfer(address recipient, uint256 amount) public virtual override returns (bool) {
        uint256 fee = amount / 10; // 10% fee
        uint256 transferAmount = amount - fee;
        super.transfer(recipient, transferAmount);
        super.transfer(address(this), fee);
        return true;
    }
}

2.Add the fee-on-transfer token to the path in the _swapSupportingFeeOnTransferTokens function:

// Exploit code
FeeOnTransferToken feeToken = new FeeOnTransferToken();
address[] memory path = new address[](2);
path[0] = address(feeToken);
path[1] = address(USDT); // Assuming USDT is another token in the path
_swapSupportingFeeOnTransferTokens(path, msg.sender);

By adding the fee-on-transfer token to the path, the attacker can exploit the vulnerability and receive more tokens than expected in the swap due to the incorrect calculation of amountInput.

Impact

This vulnerability can be exploited to create arbitrage opportunities, enabling attackers to siphon off value from the contract, ultimately resulting in financial losses for users and undermining the integrity of the contract's operations.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/main/jalaswap-dex-contract/contracts/JalaRouter02.sol#L352-L374

Tool used

Manual Review

Recommendation

To fix this issue, we need to consider the scenario where the token being swapped is a fee-on-transfer token. One way to handle this is by using the transferFrom function to move the tokens from the sender to the contract before performing the swap. This ensures that the contract has the correct amount of tokens available for swapping.

Here is an example of a patch code to fix the vulnerability:

function _swapSupportingFeeOnTransferTokens(address[] memory path, address _to) internal virtual {
    for (uint256 i; i < path.length - 1; i++) {
        (address input, address output) = (path[i], path[i + 1]);
        (address token0, ) = JalaLibrary.sortTokens(input, output);
        IJalaPair pair = IJalaPair(JalaLibrary.pairFor(factory, input, output));

        uint256 amountInput;
        uint256 amountOutput;

        {
            // scope to avoid stack too deep errors
            (uint256 reserve0, uint256 reserve1, ) = pair.getReserves();
            (uint256 reserveInput, uint256 reserveOutput) = input == token0
                ? (reserve0, reserve1)
                : (reserve1, reserve0);

            // Transfer tokens from sender to contract
            IERC20(input).transferFrom(msg.sender, address(this), amountInput);

            amountInput = IERC20(input).balanceOf(address(this));
            amountOutput = JalaLibrary.getAmountOut(amountInput, reserveInput, reserveOutput);
        }

        (uint256 amount0Out, uint256 amount1Out) = input == token0
            ? (uint256(0), amountOutput)
            : (amountOutput, uint256(0));

        address to = i < path.length - 2 ? JalaLibrary.pairFor(factory, output, path[i + 2]) : _to;
        pair.swap(amount0Out, amount1Out, to, new bytes(0));
    }
}

In this patch code, we added the IERC20(input).transferFrom(msg.sender, address(this), amountInput); line before calculating the amountInput to ensure that the contract has the correct amount of tokens available for swapping. This change helps to handle fee-on-transfer tokens properly and prevents the vulnerability of insufficient handling of fee-on-transfer tokens in the _swapSupportingFeeOnTransferTokens function.

nevillehuang commented 7 months ago

Invalid, all balances before and after are checked whenever _swapSupportingFeeOnTransferTokens is invoked, as seen in an example here. No arbitrage opportunity is possible given the transfer fees are paid accordingly to relevant parties