Malicious user can steal the tokens of other users to mint and buy GSP shares

Summary

When a user wants to mint shares from the GSP, they have to transfer tokens to the contract, first before they can mint, which means the amount of minted shares they are going to receive, is based on the calculations done in the buyShares function, but this leads to a situation where, if multiple users transfer tokens to the GSP and did not call buyShares immediately, the latest user to transfer and call the function will get minted the shares meant for the other users that transferred base and quote tokens earlier.

Vulnerability Detail

Lets take a look at the Function buyShares and see how this kind of vunerability will affect other users

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L25C4-L82C6

 function buyShares(address to)
        external
        nonReentrant
        returns (
            uint256 shares,
            uint256 baseInput,
            uint256 quoteInput
        )
    {

        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)) - _MT_FEE_BASE_;
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)) - _MT_FEE_QUOTE_;
       //@audit here we can see that the way the tokens are calculated is by checking the balance of the contract to know how much of   both tokens it has 

        uint256 baseReserve = _BASE_RESERVE_;
        uint256 quoteReserve = _QUOTE_RESERVE_;
     //@audit the reserves are gotten, which actually implies the set reserves function during the last action taken, this effectively tracks how much the contract had before 

        baseInput = baseBalance - baseReserve;
        quoteInput = quoteBalance - quoteReserve;
     //@audit The issue shows up over here as to get how much tokens the shares are to be minted for, which means it doesnt have to be your token, if a couple of people transfer to the contract, and you mint before them, you effectively get their shares and they get noting,

When We analyze the function properly we Look at the notes and instructions which lays a path on how the shares are minted, as shown below

    /// @notice User mint Lp token and deposit tokens, the result is rounded down
    /// @dev User first transfer baseToken and quoteToken to GSP, then call buyShares

Users Have to Transfer tokens to the Contract first before they can actually buy shares, which leads to the implementation error i described.

Impact

The Loss of Funds for users that did not call buyShares after transferring, and also the effective stealing of tokens by a malicuous user.

Code Snippet

Tool used

Manual Review

Recommendation

A More Accurrate Depsoiting System Should be implemented, like a deposit function, which tracks how much a user has deposited and effectively uses that to calculate the base and quote input, that the shares are to be minted from.

Duplicate of #78

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

inzinko - Malicious user can steal the tokens of other users to mint and buy GSP shares #127