Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_calc_withdraw_one_coin() allows _token_amount to be more than more than available #114

Open
hats-bug-reporter bot opened this issue Oct 16, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

Github username: --
Twitter username: --
Submission hash (on-chain): 0x78f8fa279e516cf5443ec606536ab956a551b71864159b5b1f1db1bf39343eb4
Severity: low

Description:
Description
_calc_withdraw_one_coin() function is a core function which calculates the amount receive when withdrawing a single coin,This function is used in two functions i.e

calc_withdraw_one_coin()=>public visible function where users can call and check how many tokens he/she would receive )

/**
     * @notice Calculate the amount received when withdrawing a single coin.
     * @param _token_amount: Amount of LP tokens to burn in the withdrawal
     * @param i: Index value of the coin to withdraw
     */
    function calc_withdraw_one_coin(uint256 _token_amount, uint256 i) external view returns (uint256) {
        (uint256 dy, ) = _calc_withdraw_one_coin(_token_amount, i);
        return dy;
    }

remove_liquidity_one_coin()=>users can call this function to remove liquidity one coin

/**
     * @notice Withdraw a single coin from the pool
     * @param _token_amount: Amount of LP tokens to burn in the withdrawal
     * @param i: Index value of the coin to withdraw
     * @param min_amount: Minimum amount of coin to receive
     */
    function remove_liquidity_one_coin(
        uint256 _token_amount,
        uint256 i,
        uint256 min_amount
    ) external nonReentrant {
        // Remove _amount of liquidity all in a form of coin i
        require(!is_killed, "Killed");
        (uint256 dy, uint256 dy_fee) = _calc_withdraw_one_coin(_token_amount, i);
        require(dy >= min_amount, "Not enough coins removed");

        balances[i] -= (dy + (dy_fee * admin_fee) / FEE_DENOMINATOR);
        token.burnFrom(msg.sender, _token_amount); // dev: insufficient funds
        transfer_out(coins[i], dy);

        emit RemoveLiquidityOne(msg.sender, i, _token_amount, dy);
    }

below is the _calc_withdraw_one_coin() function which holds all the logic i.e amount received when withdrawing a single coin

_calc_withdraw_one_coin()=>core logic function which calculates the receive amount

 function _calc_withdraw_one_coin(uint256 _token_amount, uint256 i) internal view returns (uint256, uint256) {
        // First, need to calculate
        // * Get current D
        // * Solve Eqn against y_i for D - _token_amount
        uint256 amp = get_A();
        uint256 _fee = (fee * N_COINS) / (4 * (N_COINS - 1));
        uint256[N_COINS] memory precisions = PRECISION_MUL;
        uint256 total_supply = token.totalSupply();

        uint256[N_COINS] memory xp = _xp();

        uint256 D0 = get_D(xp, amp);
        uint256 D1 = D0 - (_token_amount * D0) / total_supply;
        uint256[N_COINS] memory xp_reduced = xp;//@check-cannot withdraw more than amount not checked

        uint256 new_y = get_y_D(amp, i, xp, D1);
        uint256 dy_0 = (xp[i] - new_y) / precisions[i]; // w/o fees

        for (uint256 k = 0; k < N_COINS; k++) {
            uint256 dx_expected;
            if (k == i) {
                dx_expected = (xp[k] * D1) / D0 - new_y;
            } else {
                dx_expected = xp[k] - (xp[k] * D1) / D0;
            }
            xp_reduced[k] -= (_fee * dx_expected) / FEE_DENOMINATOR;
        }
        uint256 dy = xp_reduced[i] - get_y_D(amp, i, xp_reduced, D1);
        dy = (dy - 1) / precisions[i]; // Withdraw less to account for rounding errors

        return (dy, dy_0 - dy);
    }

issue: we we look into this function ,this function does not verify that the _token_amount should be always smaller than xp[i] i.e the function does not check :

tokenAmount <= xp[tokenIndex]

due to this ,the function allow tokenAmount to be greater than xp[i]

Attachments

  1. Proof of Concept (PoC) File

Impact:

  1. user may get incorrect or more amount when calling the calc_withdraw_one_coin()
  2. this function is also called in remove_liquidity_one_coin but ,here there will be no impact as if the _token_amount is greater then the function will revert with below
    token.burnFrom(msg.sender, _token_amount);

so as this issue does not lead to loss of funds but the users may get incorrect or more amount when calling the calc_withdraw_one_coin()=>public visible function where users can call and check how many tokens he/she would receive

therefore, i am submitting this issue as low

  1. Revised Code File (Optional)

implement the below fix in the _calc_withdraw_one_coin() logic function

   require(_token_amount <= xp[i], "Cannot withdraw more than available");
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Oct 16, 2024
@Ghoulouis
Copy link

The transaction read returns wrong result due to user entering wrong input is user error.

@Ghoulouis Ghoulouis added the invalid This doesn't seem right label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant