sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

J4de - [High] `dripDuration` should be controlled by the caller rather than a fixed value #78

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

J4de

high

[High] dripDuration should be controlled by the caller rather than a fixed value

Summary

Taurus prevents sandwich attacks by adding claimed fees to yields over time. The process for users to get yield is as follows:

  1. User deposits collater to vault, the cumulativeTauRewardPerCollateral is TauPerColl0
  2. Keeper call contract UniswapSwapAdapter's swap function to swap TAU with GLP
  3. The TAU obtained by the swap will continue to be included in the reward TAU for 1 day
  4. The cumulativeTauRewardPerCollateral is TauPerColl1 after 1 day
  5. The user's yield is (TauPerColl1 - TauPerColl0) * Collater

The above process can run normally when Keeper calls swap every day. But if the keeper calls swap for more than 1 day, for example, 2 days, then the attacker can share the rewards of 2 days of staking by everyone with only 1 day of staking.

An attacker can cause a swap tooMuchSlippage error or insufficient TAU in the market by purchasing a large amount of TAU.

In addition, if the dripDuration is fixed at 1 day, the yields for users whose staking time is within one day will be reduced.

Vulnerability Detail

Impact

User's yields will be reduced or shared equally by the attacker.

Code Snippet

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/TauDripFeed.sol#L73

Tool used

Manual Review

Recommendation

It is recommended that the dripDuration value is passed in by the keeper and is the actual yield time of the swap token.

Sierraescape commented 1 year ago

In addition, if the dripDuration is fixed at 1 day, the yields for users whose staking time is within one day will be reduced.

This is fine, we want to encourage people to stake for longer than one day.

An attacker can cause a swap tooMuchSlippage error or insufficient TAU in the market by purchasing a large amount of TAU.

This also seems fine. In general, swaps can all be frontrun, but since the swap can just be run again, the attack seems rather useless. An attacker who buys lots of TAU will probably lose much more money than they make. Inflated TAU price will drive users to mint/borrow more Tau, sell it on the market, then perhaps buy GLP and do so again. The vault has a pretty robust self-correcting price mechanism which would be very expensive to fight.

The above process can run normally when Keeper calls swap every day. But if the keeper calls swap for more than 1 day, for example, 2 days, then the attacker can share the rewards of 2 days of staking by everyone with only 1 day of staking.

Swaps cost about $0.02 currently, so we plan on swapping every day. This has a variety of other benefits too, such as keeping vault yield stable to make it more fair for entering and exiting users. That said, the math isn't quite so simple. Generally about 20% of protocol yield will be sent to the protocol (to do things like incentivize LPs) rather than paying back loans, and another 0.3-0.4% will be lost to slippage as vault yield is swapped into tau. Additionally, the attacker must share total vault returns with the rest of the vault, whereas the attacker's returns will be distributed after the attacker is gone and thus must be shared among fewer participants. Some math. I haven't included the effect of esGmx rewards, but these will also decrease the efficacy of this attack vs. just staking separately.

So if my math is correct, the effectiveness of the approach increases as the time between reward distributions increases and as the attack collateral decreases as a fraction of the vault collateral. The yieldDuration must be at least 1.256 days for the attack to be profitable at all. Additionally, the attacker faces diminishing returns as their attack grows larger. If they want to steal 50% of the vault's profit from the last yieldDuration, they have to deposit over 130% of the vault's TVL, depending on how long the yieldDuration was. The attack begins to grow significant around 1.9 days, when 5% of a single day of vault yield can be stolen by a large, optimally-sized deposit of 25% of the vault's TVL. At 2 days since the last swap, the max that an attacker can steal is ~7% of a single day's yield, by depositing 25% of vault TVL.

All of this is to say that swapping once / day seems perfectly safe and has a healthy buffer before it is exploitable. Should a keeper miss a day somehow, the SwapHandler is capable of swapping a fraction of vault tokens at a time, allowing the keeper to spread earnings over subsequent days without modifying the drip duration. Allowing the drip duration to be variable is a good idea, but if Taurus ever gets very large, it could lead to increased gas costs without much benefit.

Sierraescape commented 1 year ago

Since a properly functioning keeper generally will run multiple daily swaps, any missed days can be easily handled (by swapping extra earnings for a few subsequent days), and this attack is both capital-intensive and subject to rapidly diminishing returns, it doesn't seem to be a vulnerability to the protocol.