Lines of code
<https://github.com/code-423n4/2022-03-prepo/blob/f63584133a0329781609e3f14c3004c1ca293e71/contracts/core/SingleStrategyController.sol#L32-L40&gt;
<https://github.com/code-423n4/2022-03-prepo/blob/f63584133a0329781609e3f14c3004c1ca293e71/contracts/core/SingleStrategyController.sol#L79-L81&gt;

Vulnerability details

Impact

That exploit is possible because of the implementation of the deposit function of the SingleStrategyController contract.

// Assumes approval to take _amount has already been given by vault
function deposit(uint256 _amount)
    external
    override
    onlyVault
    nonReentrant
{
    _baseToken.safeTransferFrom(_vault, address(this), _amount);
    _strategy.deposit(_baseToken.balanceOf(address(this)));
}

If an attacker manages to be the first user who deposits to the Collateral contract, he’ll get his shares minted (an amount that is equal to the amount of tokens he provided). Let’s assume he deposited the minimum amount of token which is 2, so he’ll get 1 share (because the fees will be 1). Then, he’ll front-run every call to deposit function of the Collateral contract and transfer amount of _baseToken to the SingleStrategyController so that the totalValue() will be greater than the amount of tokens the user wants to deposit. So now, when the user’s deposit will be executed, he’ll transfer the tokens, pay the fees and deposit it to the strategy (which will also deposit the tokens that the attacker had transferred before). But the user won’t get any shares minted! That’s because _shares = (_amountToDeposit * totalSupply()) / (_valueBefore); will assign 0 to _shares, because the attacker made sure that _valueBefore (which is the value of totalValue() before the deposit) is greater than _amountToDeposit, and we know that totalSupply() == 1. So the user won’t get any shares minted, but the tokens will be transferred and deposited to the strategy, and the attacker (which has all the shares) will be able to withdraw all these tokens from the strategy - by simply call withdraw with _amount = 1. That will give him uint256 _owed = (_strategyController.totalValue() * _amount) / totalSupply(); tokens, which is exactly totalValue(). Of course that attacker will have to pay fees from this withdraw, but it’s still profitable for him.

Tools Used

Remix and VS Code

Recommended Mitigation Steps

This can be fixed by changing the implementation of SingleStrategyController - by changing the deposit function to deposit exactly the amount of token recieved, or by changing the totalValue function to return only the value in the strategy.

The text was updated successfully, but these errors were encountered:

All reactions