search

sherlock-audit / 2024-05-napier-update-judging

8 stars 7 forks source link

zzykxx - Users can frontrun LSTs/LRTs tokens price increase in order to capture extra value #66

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

zzykxx

medium

Users can frontrun LSTs/LRTs tokens price increase in order to capture extra value

Summary

Users can frontrun the price increase of a supported LSTs/LRTs tokens in order to capture value.

Vulnerability Detail

Napier allows users to deposit ETH into supported LSTs/LRTs protocols via adapters in exchange for shares in the form of PT and YT tokens. The in-scope LSDs/LRTs protocols that can be deposited into are:

All of them can be subject to an instant increase in their shares value (or shares balance), mainly due to beacon chain validator rewards.

Napier allows users to deposit ETH in the protocol in exchange for PT and YT tokens instantly, without being subject to a deposit queue. This allows an attacker to frontrun a price/balance increase by doing the following:

  1. Monitor the supported LSTs/LRTs protocols and the beacon chain for an imminent share price/balance increase.
  2. Frontrun the share price/balance increase by depositing ETH in the relative LST/LRT protocol adapter via Tranche::issue(), which will call BaseLSTAdapter::prefundedDeposit()/BaseLSTAdapterUpgradeable::prefundedDeposit(), in exchange for PT and YTtokens.
  3. Wait for the share price increase to take effect, which will increase the TVL of the adapter (in terms of ETH) and in turn the value of the PT and YT tokens.
  4. Call Tranche::redeemWithYT(), which will internally call BaseLSTAdapter::prefundedRedeem()/BaseLSTAdapterUpgradeable::prefundedRedeem(), to redeem PT/YT in exchange of ETH. This is currently only possible for the EETHAdapter and the UniEthAdapter adapters.

This allows an attacker to capture profit while not being subject to the risks of staking.

On some adapters the function that triggers the price increase it's permissionless, which allows an attacker to execute all of the series of calls listed above atomically. One example of such adapter is the Renzo adapter, in which the price increase of ezETH shares (the Renzo token) is triggered by a call to DelayedWithdrawalRouter::claimDelayedWithdrawals() on Eigenlayer.

Impact

An attacker can frontrun an imminent price/balance increase of a supported LSD/LRT token in order to capture value, which translates into hontest Napier stakers earning less than they should.

Code Snippet

Tool used

Manual Review

Recommendation

Introduce a deposit queue, this prevents an attacker from frontrunning price increases for profit.

Duplicate of #65

zzykxx commented 3 months ago

Escalate

Not a duplicate of #65:

sherlock-admin3 commented 3 months ago

Escalate

Not a duplicate of #65:

  • This issue is about capturing profits, which is possible because deposits can be made immediately allowing to frontrun imminent value increases.

  • 65 is about avoiding losses, which is possible because funds can be withdrawn immediately allowing to frontrun imminent value decreases.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

massun-onibakuchi commented 3 months ago

I think the reported issue is caused by LST/LRT which are TRUSTED. Such a kind of issue is seen in many LST/LRT integrations.

Next, the actual attack flow can be more complex because Tranche charges an issuance fee on step 2 above. The attacker may need to buy tokens on the secondary market.

Last, I'm not sure how much profit the attacker can make by frontrunning a price increase of a target protocol. This is because the share price increase caused by one reward distribution may be small enough. We need to know how many percentage share price increase in average. Actually I think some of those LST/LRT themselves accept such attack vector. e.g. RocketPool rETH, they don't have a withdrawal queue. They allow users to frontrun the rETH price increase and redeem rETH for ETH immediately, in my understanding.

WangSecurity commented 3 months ago

@zzykxx correct me if I'm wrong, but both this and #65 are possible because the price change (be that drop or increase) are known before it actually happens, allowing the attacker to get more value, either deposit before the increase and withdraw after it OR withdraw before the price drop and deposit after it (the second step during the price drop is not mandatory, but potentially leade to having more funds when the price is back up). I see the difference is front-running price drops and increases, but both these actions are possible due to knowing future price before it's applied?

WangSecurity commented 3 months ago

If no answer is provided, then I believe the core issue of both this and #65 is that price changes are known before they actually happen, be that price increase or decrease. Hence, planning to reject the escalation and keep these issues as duplicates.

zzykxx commented 3 months ago

@zzykxx correct me if I'm wrong, but both this and #65 are possible because the price change (be that drop or increase) are known before it actually happens, allowing the attacker to get more value, either deposit before the increase and withdraw after it OR withdraw before the price drop and deposit after it (the second step during the price drop is not mandatory, but potentially leade to having more funds when the price is back up). I see the difference is front-running price drops and increases, but both these actions are possible due to knowing future price before it's applied?

Also withdrawing after a deposit (to frontrun a price increase) is not mandatory. The way I see it is this one:

I submitted them this way because the same issues have been judged as separate in a past contest (RioNetwork - #190, RioNetwork - #52). This is also the reason why I submitted 66 as medium severity: profits can't be captured atomically if 65 is fixed, but profits can still be captured. If the issues were to be judged as duplicates, I think there's ground for arguing that 66 is high severity because the profit-taking transactions can be executed atomically, including the one that triggers the share price increase (in some adapters), as explained in the report.

WangSecurity commented 3 months ago

Fair enough. I believe that these two indeed have the same root cause that the price is known before it's implemented, allowing to gain additional value. These two ways can be used either separate or together, leading to more gains to the attacker and more losses to other users. I agree in that sense they're sufficient to high severity. Since the escalation asked to de-dup them, planning to reject the escalation but increase severity to High.

merlin-up commented 3 months ago

@WangSecurity @zzykxx Respectfully, I don't agree that this issue is of high severity for the following reasons:

1) Monitoring the supported LSTs/LRTs protocols and the beacon chain for an imminent share price/balance increase: Essentially, the attacker must monitor the mempool to frontrun a balance increase. Monitoring the mempool is not a free activity, and the attacker will incur costs for doing so.

2) Frontrunning the share price/balance increase by depositing ETH in the relative LST/LRT protocol adapter via Tranche::issue(), which will call BaseLSTAdapter::prefundedDeposit()/BaseLSTAdapterUpgradeable::prefundedDeposit(), in exchange for PT and YT tokens: When any user deposits ETH, they pay a fee to the Napier protocol.

// Transfer fee to `feeRecipient` and the remaining underlying to the adapter
_underlying.safeTransferFrom(msg.sender, feeRecipient, fee);
_underlying.safeTransferFrom(msg.sender, address(adapter), underlyingAmount - fee);

The share price increase must be large enough to cover the costs of monitoring the mempool and the fee paid during the deposit. Therefore, it is not economically viable for the attacker to carry out this attack.

WangSecurity commented 3 months ago

Great remarks @Soloviy. Regarding the first, it can be done via MEV bots and they can be coded to take only the opportunities that will cover the fee cost (for example, when there's a large deposit into integrated protocol leading to a large price increase, or the price of other tokens that effect the price of the underlying LST/LRT). To my knowledge it's not costly and don't require to constantly sitting and looking at the mempool as it can be automated.

Hence, decision remains the same, reject the escalation, but increase the severity to High.

merlin-up commented 3 months ago

@WangSecurity Yes, that's my point. In order to automate the process of monitoring transactions in the mempool: 1) the attacker must set up an Ethereum full node: Recommended hardware requirements to run a full node: A fast CPU with 4+ cores 16 GB+ of RAM A fast SSD drive with at least 1 TB of space (storage capacity will grow over time) 25 MBit/s bandwidth 2) or access the mempool with QuickNode and pay monthly for a tariff plan.

Another significant reason why this is not economically viable for the attacker is the fee during deposit.

Lastly, the protocol can always reduce the maxStakeLimit value for the adapter contract.

I've run out of arguments as to why I consider this issue to be of medium severity. In any case, the final decision is up to you.

WangSecurity commented 3 months ago

Thank you for this quite insightful explanation of what is the cost to automate the process. The problem is that there are already lots of users who already run the Ethereum node and as far as I'm aware, setting up these MEV bots is a viable strategy that lots of node runners use to extract more value from that job. So the attacker wouldn't need to set up a node for specifically this vulnerability. It'll be the users who already run nodes and have set up MEV bots, which again to my knowledge is quite common.

Regarding the fee, sorry for the confusion, I tried to answer it in the previous comment, but since it was unclear I will elaborate. These bots can be configured to NOT take every opportunity. What I mean by that, is these bots will only take opportunities where the profit is more than the cost, i.e. when the price increase/decrease is very sharp and is larger than the fee, and will not take opportunities where the fee is larger.

About the maxStakeLimit not sure how it solves the issue, to be honest. For example, it still doesn't mean the user cannot withdraw? when it comes to deposit, if they want to mitigate the issue by reducing the the maxStakeLimit it will also block regular users from depositing. Hence, I don't think it's viable argument, but would be glad to see additional explanation.

I hope it answers the questions and explains why I believe it's suitable for being a High severity.

Decision remains the same planning to reject the escalation, but increase the severity to High.

zzykxx commented 3 months ago

The fee on deposit point raised by @Soloviy lowers the number of scenarios where an attacker can make a profit. This is not a crystal clear high severity issue and I don't feel comfortable arguing for that.

Given what @WangSecurity wrote I'm also not sure how "root cause" should be interpreted in the context of Sherlock. I always interpreted "root cause" as the core error in the code/design that allows an exploit/bug/vuln. In case of LSTs/LRTs instant price changes are not an "error", they are inevitable. At some point rewards have to be added and penalties/slashings removed.

If Napier adds a deposit queue (to fix 66) and a withdraw queue (to fix 65) the described attacks are not possible anymore, but the "root cause" (being a price drop/increase) is still there.

z3s commented 3 months ago

Yes, the root cause is the lack of a queue for withdrawals/deposits, which makes front-running LSTs/LRTs instant price changes possible.

WangSecurity commented 3 months ago

Yes, excuse me for confusion and bad phrasing. The Lead Judge made a very correct comment, both of these issues are possible because there is not queue for withdrawals/deposits which allows to gain additional value due to knowing the instant price changes. The fix for both of these issues is also the same. Hence, I believe they should remain duplicates. The high severity was explained in this comment above.

Hence, decision remains the same, planning to reject the escalation, but upgrade severity to high.

Evert0x commented 3 months ago

Result: High Duplicate of #65

sherlock-admin4 commented 3 months ago

Escalations have been resolved successfully!

Escalation status: