GSPFunding.sol

Summary

The shares in the GSPFunding.sol are calculated based on the ratio between tokens sent and the current token balance. The smaller ratio between baseSent/baseBalance and quoteSent/quoteBalance is taken and is used to mint new shares. Due to this way of calculation, if different amounts of tokens are sent, or the base and quote token decimals vary a lot, user tokens can get stuck while selling shares.

Vulnerability Detail

The first deposit of tokens calculates shares based solely on the amount of tokens sent and uses the price _I_ as a determining factor to avoid errors when selling shares. Unfortunately, all other deposits do not make use of the price and instead determine the share minting based on the ratios between sent tokens and total balance and uses the smaller ratio of the two. Since the GSPFunding.sol does not enforce amounts sent, we can send different amounts of base and quote tokens, and thus a smaller amount of shares could be unfairly minted, such that it cannot restore all of the tokens even if immediately sold, thus user tokens get stuck.

An example scenario looks like this: User A is the first depositor, he deposits 1_000_000 of both tokens, so he gets 1_000_000 shares, totalSupply = 1_000_000. User B is second, he deposits 100_000 base and 10_000 quote tokens, thus the 2 ratios are 0.1 and 0.01, so the contract takes the smaller ratio and uses it for minting, thus user B will get totalSupply * 0.01 shares, which is = 10_000 User B decides to sell his shares to redeem his tokens sent, but the calculations go as follows: base token out = base balance shares / totalSupply = 1_100_000 10_000 / 1_010_000, so he will get only ~10891 tokens, instead of the 100_000 he deposited, so he gets ~90% of his initial deposit stuck in the contract and since he burned his shares, he has no way of redeeming them. The reason for this is mainly the usage of ratios. The contract does not enforce on the buyer to deposit equal amounts of both tokens and even if it did, decimal differences between the base and quote tokens could reap the same problems. And written POC is provided below. It will fail due to the user having burned all of his shares, proving the tokens are stuck. The logs show the token balance of the GSP to prove the stuck amount:

function test_tokensStuck() public {
        MockERC20 mockBaseToken;
        MockERC20 mockQuoteToken;

        mockBaseToken = new MockERC20("mockBaseToken", "mockBaseToken", 18);
        mockBaseToken.mint(USER, 1e18);
        mockBaseToken.mint(OTHER, 1e18);

        mockQuoteToken = new MockERC20("mockQuoteToken", "mockQuoteToken", 18);
        mockQuoteToken.mint(USER, 1e18);
        mockQuoteToken.mint(OTHER, 1e18);

        GSP gspTest = new GSP();
        gspTest.init(
            address(10),
            address(mockBaseToken),
            address(mockQuoteToken),
            0,
            0,
            1000000,
            500000000000000,
            false
        );

        vm.startPrank(USER);
        mockBaseToken.transfer(address(gspTest), 1_000_000);
        mockQuoteToken.transfer(address(gspTest), 1_000_000);
        gspTest.buyShares(USER);
        console.log("After the first deposit of 1 million each token:");
        console.log("Balance of base token: ", IERC20(gspTest._BASE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("Balance of quote token: ", IERC20(gspTest._QUOTE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("#########################");
        vm.stopPrank();

        vm.startPrank(OTHER);
        mockBaseToken.transfer(address(gspTest), 100_000);
        mockQuoteToken.transfer(address(gspTest), 10_000);
        gspTest.buyShares(OTHER);
        console.log("Another user deposits 100k base and only 10k quote, so the ratio is smaller");
        console.log("Balance of base token: ", IERC20(gspTest._BASE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("Balance of quote token: ", IERC20(gspTest._QUOTE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("#########################");
        vm.stopPrank();

        vm.startPrank(OTHER);
        gspTest.sellShares(10_000, OTHER, 0, 0, "", block.timestamp + 1000);
        console.log("The second user sells all his shares, his base token gets stuck in the gsp due to ratio difference");
        console.log("Balance of base token: ", IERC20(gspTest._BASE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("Balance of quote token: ", IERC20(gspTest._QUOTE_TOKEN_()).balanceOf(address(gspTest)));
        console.log("#########################");

        gspTest.sellShares(1, OTHER, 0, 0, "", block.timestamp + 1000);
        vm.stopPrank();
    }

Impact

User losses, stuck tokens

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L65-L71 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L106-L113

Tool used

Manual Review

Recommendation

The problem comes from the possibility of big differences between tokens sent. Protocols offering LP for tokens usually offer functionality for individual token handling, but if that is not the intention, you could enforce the user to send equal amounts of both tokens, or keep shares separate.

Duplicate of #74

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

p-tsanev - GSPFunding.sol #111