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

6 stars 4 forks source link

Nyxaris - Token Truncation in Liquidity Removal #164

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 8 months ago

Nyxaris

medium

Token Truncation in Liquidity Removal

Summary

The burn function calculates the amounts of underlying tokens (amount0 and amount1) to be returned to a liquidity provider based on their share of the liquidity pool. Due to Solidity's integer division, any fractional part of the tokens calculated is truncated and not returned to the user, potentially leading to a small loss of value known as "dust."

Vulnerability Detail

Assume _totalSupplyof LP tokens is 1000. A user wants to burn 50 LP tokens (liquidity). The contract holds 1005 tokens of _token0 (balance0) and 2010 tokens of _token1 (balance1). The expected amount0 to be returned is 50.25 tokens (50 / 1000 1005). The expected amount1 to be returned is 100.5 tokens (50 / 1000 2010). Solidity's integer division truncates the results to 50 tokens for amount0 and 100 tokens for amount1. 0.25 tokens of `_token0 0.25 tokens of _token0 and 0.5 tokens of _token1 are not returned to the user and remain in the contract as "dust."

Impact

While the impact of truncation on individual transactions is minimal, the cumulative effect can lead to a noticeable amount of tokens being retained by the contract over time.

Code Snippet

code

function burn(address to) external lock returns (uint256 amount0, uint256 amount1) {
        (uint112 _reserve0, uint112 _reserve1, ) = getReserves(); // gas savings
        address _token0 = token0; // gas savings
        address _token1 = token1; // gas savings
        uint256 balance0 = IERC20(_token0).balanceOf(address(this));
        uint256 balance1 = IERC20(_token1).balanceOf(address(this));
        uint256 liquidity = balanceOf[address(this)];

        bool feeOn = _mintFee(_reserve0, _reserve1);
        uint256 _totalSupply = totalSupply; // gas savings, must be defined here since totalSupply can update in _mintFee
        amount0 = (liquidity * balance0) / _totalSupply; // using balances ensures pro-rata distribution
        amount1 = (liquidity * balance1) / _totalSupply; // using balances ensures pro-rata distribution
        if (amount0 == 0 || amount1 == 0) revert InsufficientLiquidityBurned();
        _burn(address(this), liquidity);
        _safeTransfer(_token0, to, amount0);
        _safeTransfer(_token1, to, amount1);
        balance0 = IERC20(_token0).balanceOf(address(this));
        balance1 = IERC20(_token1).balanceOf(address(this));

        _update(balance0, balance1, _reserve0, _reserve1);
        if (feeOn) kLast = uint256(reserve0) * reserve1; // reserve0 and reserve1 are up-to-date
        emit Burn(msg.sender, amount0, amount1, to);
    }

Tool used

Manual Review

Recommendation

Rounding Mechanism Review: Consider implementing a rounding mechanism that can fairly distribute the truncated amounts, possibly by periodically redistributing accumulated dust to liquidity providers or adding it back to the pool's reserves. Contract Upgrade: If the accumulated dust becomes economically significant, upgrade the contract to handle these small balances more effectivel

nevillehuang commented 7 months ago

Invalid, shares same root cause as #50 but impact is insignificant since both liquidity and balance0 are sacaled to 18 decimals, so the example used is incorrect