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

6 stars 5 forks source link

inzinko - User Will loose their tokens if they deposit the same amount of Quote token as the Maintainer Fee #139

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

inzinko

high

User Will loose their tokens if they deposit the same amount of Quote token as the Maintainer Fee

Summary

Due to the way the buyShares Function is written when a user transfers their tokens into the contract, the maintainer fee is subtracted from the balance to account for it, but in a particular situation, where the depositor sends the same amount of quote tokens to the GSP contract, they will get zero shares and will loose their tokens.

Vulnerability Detail

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

In the buyShares function, the calculation of how many shares to mint for the user is shown below,

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 Over here the issue presents itself, when the depositor, sends tokens to the contract and the balance is calaculated this, way if the user sends the same amount of Maintaner fee for the quote token, the quote balance will essentially be zero 

        uint256 baseReserve = _BASE_RESERVE_;
        uint256 quoteReserve = _QUOTE_RESERVE_;

        baseInput = baseBalance - baseReserve;
        quoteInput = quoteBalance - quoteReserve;
//@audit When the input is calculated the base input is calculated normally, as the user sent a normal amount, but over here the quote input will essentially be zero as there is no reserves yet

        require(baseInput > 0, "NO_BASE_INPUT");
//@audit The Require statement does not check for the value of quote token before proceeding and continues the function, 

But because of how the function works it will not revert but mint zero shares, for the user, The calculations uses mint ratio and if the quote ratio is zero the amount of shares to mint will also be zero. The snippet below will show reason why the function will mint zero

   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) {

            uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
            uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
            uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
            shares = DecimalMath.mulFloor(totalSupply, mintRatio);

            _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) + (DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)));
            _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) + (DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)));
        }

Impact

Loss of Quote Tokens For The User

Code Snippet

Tool used

Manual Review

Recommendation

A More Robust Deposit system should be implemented, like a check to make sure the amount of quote tokens sent to the contract is not the same as the Maintainer fee, and also a value that updates as the Maintainer fee increases to check if the tokens sent are equals to to the maintainer fee, to prevent loss of funds

nevillehuang commented 6 months ago

Invalid, buyer input error, it is their responsibility to transfer sufficient funds to cover maintainer fee and mint shares