Sturdy contest - dipp's results

Lines of code

GeneralVault.sol#L100-L128 LidoVault.sol#L122-L149

Vulnerability details

Impact

In the _withdrawFromYieldPool function in LidoVault.sol, the receivedETHAmount value is returned before the sent value for the call function is checked on line 142.

When withdrawCollateral is called in LidoVault.sol and the call fails, the transaction does not revert and the user loses the assets withdrawn from the lending pool. The withdrawn assets stay in the contract.

Proof of Concept

1. Call withdrawCollateral from LidoVault.sol with _asset = 0x0000000000000000000000000000000000000000, anything for _amount and a valid _to address.

2. The _amount of stETH is withdrawn from the lending pool.

3. _withdrawFromYieldPool is called with the stETH amount withdrawn as its _amount value.

4. In _withdrawFromYieldPool, after the stETH is swapped for ETH, a call to _to is made and the ETH amount received from the swap is sent to the address.

5. If the call fails, for example the receiving contract does not implement receive or a fallback to receive ETH, then the call should return false and the withdraw transaction should revert. But it does not revert, instead the amount of ETH received from the swap is returned from the _withdrawFromYield function. The ETH acquired from the swap remains in the contract.

Recommended Mitigation Steps

Swap line 141 with line 142 in LidoVault.sol.

Lines of code

GeneralVault.sol#L75-L89 LidoVault.sol#L79-L104

Vulnerability details

Impact

When the depositCollateral function is used in the LidoVault.sol contract, a user depositing stETH may mistakenly have a non-zero msg.value. The ETH would be passed to the contract and the user would lose that ETH.

Proof of Concept

1. Call the depositCollateral function in the LidoVault.sol contract with _asset as the stETH token address (LIDO), _amount as any withdrawable amount and msg.value as any amount more than 0.

2. The amount of msg.value in ETH is sent from the msg.sender to LidoVault.sol.

3. When _depositToYieldPool is called in the depositCollateral function the _amount of stETH will be sent to LidoVault.sol.

4. Only the _amount of stETH is deposited into the lending pool, since the ETH sent is not swapped for stETH. The ETH sent stays in the contract.

Recommended Mitigation Steps

Consider reverting the transaction when the msg.value is not 0 and the _asset value is equal to LIDO.

On line 97 of LidoVault.sol, the condition could change to require(_asset == LIDO && msg.value == 0, Errors.VT_COLLATERAL_DEPOSIT_INVALID).

1. Wrong amount emitted

Line References

GeneralVault.sol#L88

Impact

When depositing ETH in the depositCollateral function in GeneralVault.sol (when using the LidoVault.sol contract), the _amount in the emitted event may not be the same as the msg.value.

The assumption is that the DepositCollateral event should emit the actual amount deposited.

Consider using the _stAssetAmount value (returned from the _depositToYieldPool function) for the emit instead of the _amount value.