MalfurionWhitehat - Pool is vulnerable to inflation attack

[github-actions]

MalfurionWhitehat

high

Pool is vulnerable to inflation attack

Summary

In empty (or nearly empty) Pools, deposits are at high risk of being stolen through frontrunning with a "donation" to the vault that inflates the price of a share, as deposit(uint amount) does not have slippage control. This is variously known as a donation or inflation attack and is essentially a problem of slippage from ERC-4626 vaults.

Vulnerability Detail

Proof of concept:

A malicious early user can deposit() with 1 wei of LOAN_TOKEN as the first depositor of the Vault and get 1 wei of shares token (Pool token).

Then the attacker can send 10_000e18-1 of LOAN_TOKEN and inflate the price per share from 1.0000 to an extreme value of 1.0000e22 (from (1+10_000e18-1)/1).

As a result, the future user who deposits 19_999e18 will only receive 1 wei (from 19_999e18*1/10_000e18) of shares.

They will immediately lose 9999e18 or half of their deposit if they withdraw() right after the deposit().

Impact

Attacker is able to steal user's shares

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Consider requiring a minimal amount of share tokens to be minted for the first minter, and send a part of the initial mint as a permanent reserve so that the price per share can be more resistant to manipulation. OpenZeppelin calls this "virtual shares" on their attack mitigation:

OpenZeppelin/openzeppelin-contracts#3979

Duplicate of #125