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

6 stars 5 forks source link

Ragnark_323 - due to lack of proper logic for `buyShares()` function,attacker can steal the user funds silently #90

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Ragnark_323

high

due to lack of proper logic for buyShares() function,attacker can steal the user funds silently

Summary

in GSPFuding.sol i found that buyShares() function has no proper access control/logic, by frontrunning attacker can steal the user tokens by minting the LP tokens to his address and sell them using sellShares() function.in this case user completely lose his base and quote tokens

Vulnerability Detail

GSPFuding.sol contract has two important functions i.e buyShares() and sellShares() functions, by using this user can mint the Lp tokens by deposting tokens(base and quote) and burn their lp tokens and withdraw their tokens.for the buyShares() function User first transfer baseToken and quoteToken to GSP, then call buyShares to mint the Lp tokens . the amount of tokens transferred by user is directly calculated by

 // The balance of baseToken and quoteToken should be the balance minus the fee
        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)) - _MT_FEE_BASE_;
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)) - _MT_FEE_QUOTE_;

        // The reserve of baseToken and quoteToken
        uint256 baseReserve = _BASE_RESERVE_;
        uint256 quoteReserve = _QUOTE_RESERVE_;

        // The amount of baseToken and quoteToken user transfer to GSP
        baseInput = baseBalance - baseReserve;
        quoteInput = quoteBalance - quoteReserve;

.after transfering the tokens to GSP ,the buyShares() funciton is called by the user.as the tokens are already transferred to GSP the attacker observes the pending trasaction of buyShares() function which is called by user and by frontrunning(as the function does not check who transferred the tokens,it only check the difference in the Balance and reserve ) and passing the attacker address to buyShares(to) with higher gas price, attacker mints the Lp tokens without transfering the tokens himself and the user tokens are lost and get's nothing in return

Exploit scenario/POC:

1)user transfers the base and quote tokens to Gsp contract 2)user call the 'buyShares()' function 3)attackers observes the pending transaction,and call's the buyShares() function with his address and with higher gas price 4)attackers mint's the Lp tokens and sell the LP tokens using sellShares() and receive the base and quote tokens 5)user transaction will be reverted as require(baseInput > 0, "NO_BASE_INPUT"); fails

Impact

User tokens can be steal by the attacker.

Code Snippet

link for code

Tool used

Vs code,Manual Review

Recommendation

try to implement logic for transfering tokens from the user within the function, like using safeTransferFrom

token.safeTransferFrom(msg.sender,address(this),_amount);

Duplicate of #78