hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

GNU General Public License v3.0
0 stars 0 forks source link

Incorrect precision handling in Calc_Token_Amount() function can leads to unexpected Amount of Lp tokens received #98

Open hats-bug-reporter[bot] opened 3 weeks ago

hats-bug-reporter[bot] commented 3 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x3461aba3368837af526e8e7b19af55f42b038c0ac523dfc34d5bf7072fe17865 Severity: medium

Description: Description\ the calc_token_amount() function in StableSwapTwoPool.sol contract Calculates addition or reduction in token supply from a deposit or withdrawal and it returns the expected amount of LP tokens received.but here the function did not consider for the decimals of tokens(like lets say coin 1 with 18 decimals and token 2 with 6 decimals)here this function directly adds or sub the amount without considering for precision.i.e this function does not explicitly handle precision differences between tokens with varying decimals places.

Attack Scenario\


 /**
     * @notice Calculate addition or reduction in token supply from a deposit or withdrawal
     * Returns the expected amount of LP tokens received. 
     * This calculation accounts for slippage, but not fees.
     * @param amounts: Amount of each coin being deposited
     * @param deposit: Set True for deposits, False for withdrawals
     */
    function calc_token_amount(uint256[N_COINS] memory amounts, bool deposit) external view returns (uint256) {
        /**
        Simplified method to calculate addition or reduction in token supply at
        deposit or withdrawal without taking fees into account (but looking at
        slippage).
        Needed to prevent front-running, not for precise calculations!
        */
        uint256[N_COINS] memory _balances = balances;
        uint256 amp = get_A();
        uint256 D0 = get_D_mem(_balances, amp);
        for (uint256 i = 0; i < N_COINS; i++) {
            if (deposit) {
                _balances[i] += amounts[i];//@audit= does not scale for precision i.e to 1e18?
            } else {
                _balances[i] -= amounts[i];
            }
        }
        uint256 D1 = get_D_mem(_balances, amp);
        uint256 token_amount = token.totalSupply();
        uint256 difference;
        if (deposit) {
            difference = D1 - D0;
        } else {
            difference = D0 - D1;
        }
        return (difference * token_amount) / D0;
    }

lets understand the issue with example below

Attachments

  1. Proof of Concept (PoC) File
for (uint256 i = 0; i < N_COINS; i++) {
    if (deposit) {
        _balances[i] += amounts[i];
    } else {
        _balances[i] -= amounts[i];
    }
}

Scenario

Initial State

User Action

A user wants to deposit:

Current Implementation (Without Precision Handling)

Balances Update:

Problem

The current implementation treats 1 unit of Token A and 1 unit of Token B as equivalent, which is incorrect due to their different decimal places. This results in an inaccurate calculation of the pool's invariant and, consequently, the LP tokens.

Expected Implementation (With Precision Handling)

  1. Precision Adjustment:
  1. Balances Update:
  1. New Balances:

Token A: 1,000 10^18 + 1 10^18 = 1,001 10^18 Token B: 2,000 10^6 + 1 10^12 = 2,001 10^6

  1. Revised Code File (Optional)
for (uint256 i = 0; i < N_COINS; i++) {
    uint256 adjustedAmount = (amounts[i] * RATES[i]) / PRECISION; // Adjust for precision
    if (deposit) {
        _balances[i] += adjustedAmount;
    } else {
        _balances[i] -= adjustedAmount;
    }
}
omega-audits commented 3 weeks ago

There is not concept of a "unit" in the contract, they operator on the native representation of the tokens. Merely sayign that this is "incorrect" is not really a helpful description of the issue.

Can you provide a sceneario or PoC where a depositor recieves an "unexpected aount of lp tokens" as described in the title?