Sturdy contest - hickuphh3's results

Lines of code

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

Vulnerability details

Details & Impact

The require check is performed after exiting the function, meaning that the ETH transfer’s validity check is skipped. This would thus cause invalid withdrawals to be erroneously processed as valid.

Recommended Mitigation Steps

Swap the require check and the return statement

require(sent, Errors.VT_COLLATERAL_WITHDRAW_INVALID);
return receivedETHAmount;

Lines of code

https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L124 https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L179-L186 https://github.com/code-423n4/2022-05-sturdy/blob/78f51a7a74ebe8adfd055bdbaedfddc05632566f/smart-contracts/YieldManager.sol#L48 https://github.com/sturdyfi/code4rena-may-2022/blob/main/contracts/protocol/libraries/swap/UniswapAdapter.sol#L41-L76 https://github.com/sturdyfi/code4rena-may-2022/blob/main/contracts/protocol/libraries/swap/UniswapAdapter.sol#L120-L127

Vulnerability details

Details & Impact

All harvested yield tokens are swapped through the Uniswap adapter to USDC. While out of scope, the Uniswap adapter code is relevant here, as I note that the path taken for the swap would be assetFrom -> WETH -> assetTo unless assetFrom is already WETH.

The main issue is the assumption and usage of the 1% fee pool regardless of asset. Those that are familiar with UniV3 will know that there are multiple pool tiers for the same asset pair. Hence, it is possible that there are other pools (Eg. the pool with 0.3% fee) where majority of the liquidity lies instead.

Furthermore, it could be possible that the pool with 1% fee isn’t created. Thus, attempts to swap via the specified path will fail, breaking yield distribution.

Examples

WETH -> USDC

The yield manager swaps yield tokens (CRV, CVX + any additional rewards) to WETH, then to USDC, which is then swapped for other stablecoins if needed. The WETH / USDC 1% fee pool has ~$10.25M liquidity while the 0.05% fee pool has ~$250M liquidity, thus causing yield rewards to lose out due to pool slippage.

poolId 4: SUSD

The SUSD pool (Convex booster pool id 4) distributes SNX as an extra reward. The SNX/ETH 0.3% fee pool has ~$1.78M liquidity while the 1% fee pool has merely ~$36k liquidity (see Uniswap stats).

rKP3R

rKP3R is distributed as an extra reward for a number of pools. There isn’t a rKP3R / ETH pool created.

Recommended Mitigation Steps

Enable skipping conversion for some assets, and have a mapping of the pool fee as part of the customisable configuration so that pools with the best liquidity can be used.

L-01: _depositYield() doesn’t use SafeERC20 for approvals

L-02: Use safeIncreaseAllowance() instead of safeApprove()

L-03: Verify curveLPToken matches with Convex booster’s convexPoolId

L-04: ConvexCurveLPVault: Ensure 0 msg.value in _depositToYieldPool()

L-05: LidoVault: Restrict ETH sender to WETH contract

NC-01: Spelling errors

• mount → amount
• _interalToken → _internalToken

G01: ConvexCurveLPVault: Save result of external call

Line References

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

Description

The external call to retrieve the symbol of _lpToken is performed twice. To save gas, save the result of the first call into memory.

Recommended Mitigation Steps

string memory lpTokenSymbol = IERC20Detailed(_lpToken).symbol());
SturdyInternalAsset _interalToken = new SturdyInternalAsset(
  string(abi.encodePacked('Sturdy ', lpTokenSymbol),
  string(abi.encodePacked('c', lpTokenSymbol),
  IERC20Detailed(_lpToken).decimals()
);

G02: ConvexCurveLPVault: Save _lpToken.decimals() to avoid external call in pricePerShare()

Line References

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

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

Description

The _internalToken takes on the decimals of the _lpToken. Gas can be saved by replacing the external call to _internalToken for retrieval of the token decimal value in the pricePerShare() function with an SLOAD instead.

uint256 internal decimals;

// in setConfiguration()
decimals = IERC20Detailed(_lpToken).decimals();
SturdyInternalAsset _interalToken = new SturdyInternalAsset(
  string(abi.encodePacked('Sturdy ', IERC20Detailed(_lpToken).symbol())),
  string(abi.encodePacked('c', IERC20Detailed(_lpToken).symbol())),
  decimals
);

function pricePerShare() external view override returns (uint256) {
  return 10**decimals;
}