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

6 stars 5 forks source link

0xmystery - Lack of On-Chain Deadline and Slippage Protection in the buyShares Function of the GSPFunding Contract #141

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

0xmystery

medium

Lack of On-Chain Deadline and Slippage Protection in the buyShares Function of the GSPFunding Contract

Summary

The buyShares function in the GSPFunding smart contract lacks critical on-chain protections, namely deadline enforcement and slippage control. This vulnerability poses significant risks, particularly when users interact directly with the smart contract, bypassing any off-chain safeguards provided by the user interface on the official website.

Vulnerability Detail

In its current form, the buyShares function allows users to mint LP tokens by depositing base and quote tokens. However, this function does not include on-chain mechanisms to enforce a transaction deadline or control slippage. While the official website's user interface might offer off-chain slippage tolerance settings, these do not protect users who interact directly with the contract.

Off-chain slippage settings are pre-transaction checks and are not enforceable on the blockchain. As a result, in cases of direct smart contract interaction or delays in transaction execution, users are exposed to the risk of executing transactions with higher-than-acceptable slippage.

As can be seen in buyShares() below, shares minted/returned could be not in favor to the caller primarily due to a lower than expected mintRatio calculated:

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

        // [Code for calculating shares, baseInput, and quoteInput]

        if (totalSupply == 0) {
            // case 1. initial supply
            // The shares will be minted to user
            shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_)
                ? DecimalMath.divFloor(quoteBalance, _I_)
                : baseBalance;
            // The target will be updated
            _BASE_TARGET_ = uint112(shares);
            _QUOTE_TARGET_ = uint112(DecimalMath.mulFloor(shares, _I_));
        } else if (baseReserve > 0 && quoteReserve > 0) {
            // case 2. normal case
            uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
            uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
            uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
            // The shares will be minted to user
            shares = DecimalMath.mulFloor(totalSupply, mintRatio);

            // The target will be updated
            _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) + (DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)));
            _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) + (DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)));
        }
        // The shares will be minted to user
        // The reserve will be updated
        _mint(to, shares);
        _setReserve(baseBalance, quoteBalance);
        emit BuyShares(to, shares, _SHARES_[to]);

Impact

The absence of on-chain slippage and deadline protections can lead to high-impact issues:

  1. Unfavorable Execution for Direct Interactors: Users interacting directly with the smart contract are not protected against significant slippage, leading to potentially large financial losses, especially in volatile markets.
  2. Reliance on Off-Chain Safeguards: Sole reliance on off-chain measures provided by the website interface can be inadequate for ensuring fair and expected transaction outcomes.
  3. Advanced Knowledge Requirement: Users need a higher level of blockchain proficiency to understand and mitigate the risks of direct contract interaction, potentially alienating less experienced users.

Code Snippet

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

Tool used

Manual Review

Recommendation

To address this vulnerability, the following measures are recommended:

  1. Implement On-Chain Slippage Control: Introduce a slippage tolerance parameter within buyShares(). Transactions should revert if the actual slippage exceeds this predefined tolerance.
  2. Introduce Deadline Parameter: Add a deadline parameter to the function to ensure the transaction is executed within a user-acceptable timeframe. Transactions should revert if the current block timestamp is beyond this deadline.

Duplicate of #109