Sturdy contest - Picodes's results

Lines of code

https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L100

Vulnerability details

Impact

When withdrawing collateral, user may occur a loss because of the swapping of the staked asset back to the asset. There is a build-in slippage protection here but the user has no control over it. This could easily lead to loss of user funds if they are not aware of this functionality or would have like to use a lower slippage.

Proof of Concept

We've recently witness some panic movements of stETH and its depeg on Curve, which would have certainly lead to losses for Sturdy users if the vault was live without slippage protection.

Recommended Mitigation Steps

Pass the slippage as a parameter or add an other function to do so.

Lines of code

https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L48 https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L184

Vulnerability details

Impact

In YieldManager, UNISWAP_FEE is hardcoded, which reduce significantly the possibilities and will lead to non optimal routes. In particular, all swaps using ETH path will use the wrong pool as it will use the ETH / USDC 1% one due to this line.


Proof of Concept

For example for CRV / USDC, the optimal route is currently CRV -> ETH and ETH -> USDC, and the pool ETH / USDC with 1% fees is tiny compared to the ones with 0.3 or 0.1%. Therefore using the current implementation would create a significant loss of revenue.

Recommended Mitigation Steps

Basic mitigation would be to hardcode in advance the best Uniswap paths in a mapping like it’s done for Curve pools, then pass this path already computed to the swapping library. This would allow for complex route and save gas costs as you would avoid computing them in swapExactTokensForTokens.

Then, speaking from experience, as distributeYield is onlyAdmin, you may want to add the possibility to do the swaps through an efficient aggregator like 1Inch or Paraswap, it will be way more optimal.

[Low - 01] - Slippage is incoherent in LidoVault.

In LidoVault, the slippage when swapping stETH to ETH is 2% whereas in GenericVault it is 1%. This should be unified.

Code: https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L130 https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L125

[Non-Critical - 01]

Incorrect Comment: https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L77