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

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

Open
hats-bug-reporter bot opened this issue Oct 13, 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): 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.
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Oct 13, 2024
@omega-audits
Copy link

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)

@omega-audits omega-audits added the invalid This doesn't seem right label Oct 14, 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