sherlock-audit / 2024-01-napier-judging

8 stars 5 forks source link

xMxAxMx - decreasing scale lead to loss of PT token value and creates an arbitrage opportunity #15

Closed sherlock-admin closed 5 months ago

sherlock-admin commented 5 months ago

xMxAxMx

medium

decreasing scale lead to loss of PT token value and creates an arbitrage opportunity

Summary

Users receive PT and YT tokens based on the underlying amount deposited, PT and underlying would have the same value most of the time (except after maturity ) but if the scale is decreased ( below max scale ) it creates a situation that users would receive more PT and YT tokens however this is not problematic since redeeming these tokens would still return deposited underlying but it creates an arbitrage opportunity to sell PT tokens to pool, actually users receive cheaper PT token in issuance than market price and this price difference comes from unminted yield because yield tokens wouldn't receive any yield until scale approaches max scale, decreasing scale happens when a withdrawal request is submitted on frax adapter because some fee is charged on withdraw the amount that leads to decreasing total assets and therefore scale, in this situation PT values as the underlying amount deposited.

Vulnerability Detail

Consider maxScale is 1.3 and current Scale is 1.3, now someone tries to issue some PT tokens he would receive 1000 PT and YT tokens ( we ignore fees for simplicity ) he sells yield tokens and expects to receive 1000 underlying by redeeming or selling principal ( since PT token represents principal amount ) but scale is decreased now maxScale is 1.3 and current scale is 1.2 no if he tries to redeem 1000 pt he would receive 923 underlying this happens because some part of minted shares has allocated to yield token before decreasing scale and since the scale is decreased principal has a lower value, and vise versa user that issue at this situation would receive more principal tokens ( valued as much as underlying) but this value change creates an opportunity to sell it to the pool and make a profit.

Impact

lose of principal token value arbitrage opportunity

Code Snippet

https://github.com/sherlock-audit/2024-01-napier-MehdiKarimi81/blob/3226041850e29a114ec95a722a1155e39637b89f/napier-v1/src/Tranche.sol#L206-L225 https://github.com/sherlock-audit/2024-01-napier-MehdiKarimi81/blob/3226041850e29a114ec95a722a1155e39637b89f/napier-v1/src/adapters/frax/SFrxETHAdapter.sol#L98-L113 https://github.com/sherlock-audit/2024-01-napier-MehdiKarimi81/blob/3226041850e29a114ec95a722a1155e39637b89f/napier-v1/src/adapters/frax/SFrxETHAdapter.sol#L115-L119

Tool used

Manual Review

Recommendation

Consider a specific logic for this situation

nevillehuang commented 4 months ago

Escalate

This is a duplicate of #84

sherlock-admin2 commented 4 months ago

Escalate

This is a duplicate of #84

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.

MehdiKarimi81 commented 4 months ago

Escalate

This issue indicates that PT tokens would lose value, according to docs PT represents the principal amount and shouldn't lose its value, In this case user would receive more PT tokens than the deposited amount, for example, he receives 1033 PT for depositing 1000 underlying, This creates an arbitrage opportunity enabling a user to mint PT tokens with a lower price and sell it at the nappier pool with a higher price leading to loss for liquidity providers ( since value of PT has decreased and they lose the value by arbitrage ), also after maturity users would receive less underlying than expected by redeeming PT.

I think it's a valid high I believe it can be considered a separate issue other than #84 since it highlights an impact that wasn't mentioned at #84 and requires different mitigation steps

sherlock-admin2 commented 4 months ago

Escalate

This issue indicates that PT tokens would lose value, according to docs PT represents the principal amount and shouldn't lose its value, In this case user would receive more PT tokens than the deposited amount, for example, he receives 1033 PT for depositing 1000 underlying, This creates an arbitrage opportunity enabling a user to mint PT tokens with a lower price and sell it at the nappier pool with a higher price leading to loss for liquidity providers ( since value of PT has decreased and they lose the value by arbitrage ), also after maturity users would receive less underlying than expected by redeeming PT.

I think it's a valid high I believe it can be considered a separate issue other than #84 since it highlights an impact that wasn't mentioned at #84 and requires different mitigation steps

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 4 months ago

I'm not sure the core statement. I think this is invalid or a duplicate of #84

Impact lose of principal token value: arbitrage opportunity

If Lido gets slashed and there's a change in future outlook (change in scale), it's common for market participants to trade until the price reaches what they consider fair. It's also common for a loss of the principal token (PT) to occur if the target token suffers losses (Lido is slashed, lending protocol incurs debt). Furthermore, if there are bad debts in a lending protocol (e.g. Compound), it does not necessarily mean that the principal of the Principal Token could be returned.

MxAxM commented 4 months ago

This is a duplicate of #84 as both mention the same root cause.

xiaoming9090 commented 4 months ago

Disagree that this should be a duplicate of Issue 84. Issue 84 highlighted the root cause of the lack of slippage control in the issue function, which has team can remediate by implementing a slippage control. So, that report presents a clear and straightforward actionable solution.

I do not see how this issue has to do with the lack of slippage control in the issue function, as it was not mentioned anywhere in this report. As the sponsor has mentioned, this issue is invalid.

MxAxM commented 4 months ago

Actual root cause of #84 is changing the current scale which is mentioned in this issue

MehdiKarimi81 commented 4 months ago

Disagree that this should be a duplicate of Issue 84. Issue 84 highlighted the root cause of the lack of slippage control in the issue function, which has team can remediate by implementing a slippage control. So, that report presents a clear and straightforward actionable solution.

I do not see how this issue has to do with the lack of slippage control in the issue function, as it was not mentioned anywhere in this report. As the sponsor has mentioned, this issue is invalid.

Add slippage control in the issue function is a fix for this but the root cause of this issue is changing the scale

MehdiKarimi81 commented 4 months ago

I'm not sure the core statement. I think this is invalid or a duplicate of #84

Impact lose of principal token value: arbitrage opportunity

If Lido gets slashed and there's a change in future outlook (change in scale), it's common for market participants to trade until the price reaches what they consider fair. It's also common for a loss of the principal token (PT) to occur if the target token suffers losses (Lido is slashed, lending protocol incurs debt). Furthermore, if there are bad debts in a lending protocol (e.g. Compound), it does not necessarily mean that the principal of the Principal Token could be returned.

did you mention in your docs that PT would lose its value?

Czar102 commented 4 months ago

I agree that this issue and #84 are not duplicates.

@MehdiKarimi81 why is an arbitrage opportunity considered an exploit of the system? Could you elaborate?

MehdiKarimi81 commented 4 months ago

I agree that this issue and #84 are not duplicates.

@MehdiKarimi81 why is an arbitrage opportunity considered an exploit of the system? Could you elaborate?

Consider the following scenario : State: there are 1000 ETH buffer - 1000 ETH pending claim withdrawal and 2000 shares => scale: 1, PT/Underlying ratio: 1
1 - Consider 990 ETH can be claimed through claimWithdrawal, in a normal scenario claiming would move the scale to 0.995 2 - Malicious user buys 1000 PT from Napier pool 3 - He redeems 1000 PT ( redeems with YT but we don't consider YT for simplicity ) and receives 1000 ETH ( for only PT ) 4 - He calls claimWithdrawal, since the share total supply has decreased it leverages decreasing scale, now we have 990 ETH and 1000 share so the scale would be 0.99 5 - He uses 1000 ETH to issue shares, if we ignore fees ( explained it later ) he would receive 1010 PT 6 - He sells 1000 PT on the pool to receive its underlying and benefit from 10 free PT tokens Summary: The attacker buys some PT and redeems it to leverage decreased scale however, there is a fee charged on every issuance but this can't prevent benefiting the attacker if the slashing event is considerable.

MehdiKarimi81 commented 4 months ago

In addition, as the sponsor said It's also common for a loss of the principal token (PT) to occur despite that users expect to receive their principal amount, for example, when mscale < maxScale at maturity all users that hold only PT would receive a lower amount of underlying than expected.

Czar102 commented 4 months ago

@MehdiKarimi81 I don't understand how are these points answering my question. I am concerned that this is how the system is supposed to work, the AMM liquidity providers will lose on changes in price. This is the design. I don't see why would this be a bug.

cvetanovv commented 4 months ago

I think this is completely expected behavior and the possibility of arbitration is normal and not a bug and that's why I excluded it initially. That's just how the protocol works. The sponsor also said in the comments that this report was invalid.

Czar102 commented 4 months ago

Result: Invalid Unique

sherlock-admin2 commented 4 months ago

Escalations have been resolved successfully!

Escalation status: