hats-finance / Thorn-protocol-0x1286ecdac50215a366458a14968fbca4bd95067d

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

Swappers can escape paying LP and protocol fee by swapping small amounts #100

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe1fb54e9b5a7aa6fe65a3b89934741e79a14ab5308a338bb7a236681a99db441 Severity: high

Description: Description\ Market makers provide liquidity to exchanges in exchange for a fee. For Thorn protocol, this fee is calculated and collected at StableSwapTwoPool::exchange Ln 640-649:


        uint256 dy = xp[j] - y - 1; //  -1 just in case there were some rounding errors
        uint256 dy_fee = (dy * fee) / FEE_DENOMINATOR;

        // Convert all to real units
        dy = ((dy - dy_fee) * PRECISION) / RATES[j];
        require(dy >= min_dy, "Exchange resulted in fewer coins than expected");

        uint256 dy_admin_fee = (dy_fee * admin_fee) / FEE_DENOMINATOR;
        dy_admin_fee = (dy_admin_fee * PRECISION) / RATES[j];

        // Change balances exactly in same way as we change actual ERC20 coin amounts
        balances[i] = old_balances[i] + dx;
        // When rounding errors happen, we undercharge admin fee in favor of LP
        balances[j] = old_balances[j] - dy - dy_admin_fee;

The vulnerability arises from the fact that swaps can be executed at Zero fee, translating to a loss of revenue for LP providers as well the protocol. Savvy swap customers can therefore receive more tokens than expected since fee is not deducted.

This vulnerability is also present at StableSwapThreePool::exchange Attack Scenario\ The vulnerability can be exploited by swapping small amounts. Attachments

  1. Proof of Concept (PoC) File Add this test to tests/two_pool.test.ts:

    it("Swap token0 to token1 no fees", async () => {
        let tx4 = await BUSD.approve(swap_BUSD_USDC.address, 1e10);
        let tx5 = await USDC.approve(swap_BUSD_USDC.address, 1e10);
        const expect_LP_balance = 2e6;
        let tx = await swap_BUSD_USDC.add_liquidity([1e6, 1e6], expect_LP_balance);
        let exchange_token0_balance = 1e3;
        let token1BalanceBefore = await USDC.balanceOf(accounts[0].address);
        let token1 = await swap_BUSD_USDC.balances(1);
        await swap_BUSD_USDC.exchange(0, 1, exchange_token0_balance, 0);
        let token1BalanceAfter = await USDC.balanceOf(accounts[0].address);
        let token2 = await swap_BUSD_USDC.balances(1);
        console.log(token1BalanceBefore);
        console.log(token1BalanceAfter);
        console.log(token1 - token2 - (token1BalanceAfter - token1BalanceBefore));
    });  

The last line in the POC calculates the fee collected from the swap, which is Zero

  1. Revised Code File (Optional) Consider setting a minimum swap amount or fee amount or checking that the fee collected is Non-Zero.
omega-audits commented 6 days ago

In the secenario you describe, the "attacker" is swapping dust amounts, and to the fees are so small that they round down to 0. The costs for the "attacker" here are certainaly going to be higher than what they gain in not paying fees (i.e. less than 1 wei)