Sturdy contest - 0xliumin's results

Lines of code

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

Vulnerability details

We return before the require in the LidoVault. This means we could fail to send ETH back to the user and keep executing. Marking this as high because a user could inadvertently lose all their deposited funds.

Could return the wrong deposit amount in the event

The deposit amount and msg.value aren't necessarily the same, so the amount in the event that's sent might not be representative of the amount that was actually deposited: https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/GeneralVault.sol#L88

Upgrade to Solidity 0.8.x

Solidity 0.8.x has some gas improvements that make it worth upgrading. This will also enable you to use custom errors, which also save gas.

Make ConvexCurveLPVault its own ERC20 token

You could consider making the vault an ERC20 token itself on initialization instead of creating a new contract for that. That way, you don't have to do a bunch of external calls to find out how many decimals it is, or mint them.

Use != 0 instead of > 0

This changes if you upgrade to modern solidity and use certain build tools, but != 0 is more gas efficient than > 0 https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L36

Already have WETH address stored in a variable

https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/LidoVault.sol#L59 You already have the WETH address stored in a vriable, use it here instead of pulling from the addresses provider.

Refactor for loop in getAssetYields

    for (uint256 i = 0; i < length - 1; i++) {
      assetYields[i].asset = assets[i];
      assetYields[i].amount = _totalYieldAmount.percentMul(
        volumes[i].mul(PercentageMath.PERCENTAGE_FACTOR).div(totalVolume)
	  );
      extraYieldAmount = extraYieldAmount.sub(assetYields[i].amount);
    }
    assetYields[length - 1].amount = extraYieldAmount;

That way, you don't have to do the awkward check for being on the last index.