Sturdy contest - jonah1005's results

Lines of code

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

Vulnerability details

Impact

GeneralVault.sol#L125 GeneralVault set a hardcoded slippage control of 99%. However, the underlying yield tokens price may go down. If Luna/UST things happen again, users' funds may get locked.

LidoVault.sol#L130-L137 Moreover, the withdrawal of the lidoVault takes a swap from the curve pool. 1 stEth worth 0.98 ETH at the time of writing. The vault can not withdraw at the current market.

Given that users' funds would be locked in the lidoVault, I consider this a high-risk issue.

Proof of Concept

1 stEth = 0.98 Eth

LidoVault.sol#L130-L137

Tools Used

Recommended Mitigation Steps

There are different ways to set the slippage.

The first one is to let users determine the maximum slippage they're willing to take. The protocol front-end should set the recommended value for them.

  function withdrawCollateral(
    address _asset,
    uint256 _amount,
    address _to,
    uint256 _minReceiveAmount
  ) external virtual {
      // ...
    require(withdrawAmount >= _minReceiveAmount, Errors.VT_WITHDRAW_AMOUNT_MISMATCH);
  }

The second one is have a slippage control parameters that's set by the operator.

    // Exchange stETH -> ETH via Curve
    uint256 receivedETHAmount = CurveswapAdapter.swapExactTokensForTokens(
      _addressesProvider,
      _addressesProvider.getAddress('STETH_ETH_POOL'),
      LIDO,
      ETH,
      yieldStETH,
      maxSlippage
    );

    function setMaxSlippage(uint256 _slippage) external onlyOperator {
        maxSlippage = _slippage;

        //@audit This action usually emit an event.
        emit SetMaxSlippage(msg.sender, slippage);
    }

These are two common ways to deal with this issue. I prefer the first one. The market may corrupt really fast before the operator takes action. It's nothing fun watching the number go down while having no option.

Lines of code

https://github.com/code-423n4/2022-05-sturdy/blob/main/smart-contracts/YieldManager.sol#L142-L171

Vulnerability details

Impact

YieldManager.sol#L142-L171 YieldManager distributes yield according to the current state. Big whales can deposit into the protocol before the process yield is called and left the project.

Sandwich attacks are hard to mitigate and whales can always extract value from the blockchain. Many protocols just accept this as a fact. However, there are solutions to mitigate this.

Given the mev searchers are getting more sophisticated in strategy, I consider this is a medium-risk issue.

Proof of Concept

Please refer to this tweet. bout3fiddy MEV searcher can sandwich one trade with 100M funds.

Tools Used

Recommended Mitigation Steps

A common solution to mitigate this issue is to distribute rewards pro-rata to the staking time.

I found the solution AlchemistV2 adopts really elegant. AlchemistV2 It records the underlying token's price on every deposit and distributes rewards according to the time users join the pool.

To implement such a mechanism in Sturdy is difficult though, a easier way I can think of is to use the yieldFarming mechanism and distribute yields over a period of time. AToken can pull the rewards from the yieldManager when users withdraw.