sherlock-audit / 2024-01-napier-judging

8 stars 5 forks source link

xiaoming90 - YT holders cannot receive a portion of the principal allocated by the PT holders due to the manipulation #92

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 5 months ago

xiaoming90

high

YT holders cannot receive a portion of the principal allocated by the PT holders due to the manipulation

Summary

YT holders cannot receive a portion of the principal allocated by the PT holders due to the manipulation, leading to a loss of assets for the victims.

Vulnerability Detail

On a sunny day, the YT holders can gain/earn more as they can receive a portion of the principal allocated by the PT holders. On the other hand, the PT holders will lose a portion of their principal during a sunny day.

Malicious PT holders can ensure that the Tranche is always in a "not sunny day" state upon maturity by performing the following manipulation:

1) When the Tranche and Adaptor are deployed, immediately mint the smallest possible number of shares. Attackers can do so by calling the Adaptor's prefundedDeposit function directly if they want to avoid issuance fees OR call the Tranche.issue function 2) Transfer large amounts of assets to the Adaptor directly. With a small amount of total supply and a large amount of total assets, the Adaptor's scale will be extremely large. Let the large scale at this point be $L$. 3) Trigger any function that will trigger an update to the global scale. The max scale will be updated to $L$ and locked at this large scale throughout the entire lifecycle of the Tranche, as it is not possible for the scale to exceed $L$ based on the current market yield or condition. 4) Attacker withdraws all their shares and assets via Adaptor's prefundedDeposit function or Tranche's redeem functions. Note that the attacker will not incur any fees during withdrawal. Thus, this attack is economically cheap and easy to execute. 5) After the attacker's withdrawal, the current scale of the adapter will revert back to normal (e.g. 1.0 exchange rate), and the scale will only increase due to the yield from staked ETH (e.g. daily rebase) and increase progressively (e.g., 1.0 > 1.05 > 1.10 > 1.15)

The conditions for a sunny day are as follows, where $S(t_m)$ is the max scale and $s(t_m)$ is the maturity/current scale.

$$ \frac{s\left(t_m\right)}{S\left(t_m\right)} \geqslant 1-\theta $$

Since the denominator ( $S(t_m)$ ) is an extremely large value ($L$), the $\frac{s\left(t_m\right)}{S\left(t_m\right)}$ component in the formula will be extremely small and always smaller than $1-\theta$. Thus, the Tranche is always in a "not sunny day" state.

As a result, PT holders will prevent losing $\theta$ amount of principle to YT holders.

Impact

Loss of assets for the YT holders as they cannot receive a portion of the principal allocated by the PT holders due to the manipulation.

Code Snippet

https://github.com/sherlock-audit/2024-01-napier/blob/main/napier-v1/src/Tranche.sol#L42

Tool used

Manual Review

Recommendation

Ensure that measures are in place to prevent malicious actors from manipulating the scale of the adaptor, as they impact many calculations that depend on max or maturity scales.

Duplicate of #80

massun-onibakuchi commented 4 months ago

Escalate @ydspa @nevillehuang
This issue should be low or invalid. This attack assume an attacker is able to be the first depositor to manipulate rate effectively. Also at that time, basically no one has not issued any PT/YT. Even if someone mints some PT/YT, they can redeem them by burning together (redeemWithYt) with minimum issuance fee loss.

massun-onibakuchi commented 4 months ago

The root cause is that scale can be manipulated, which is based on a kind of inflation attack. This issue can be prevented by minting some shares earlier than a attacker.

ydspa commented 4 months ago

Escalate @ydspa @nevillehuang This issue should be low or invalid. This attack assume an attacker is able to be the first depositor to manipulate rate effectively. Also at that time, basically no one has not issued any PT/YT. Even if someone mints some PT/YT, they can redeem them by burning together (redeemWithYt) with minimum issuance fee loss.

Escalate to make sponsor's disputation to be seen by Sherlock judge

sherlock-admin2 commented 4 months ago

Escalate @ydspa @nevillehuang This issue should be low or invalid. This attack assume an attacker is able to be the first depositor to manipulate rate effectively. Also at that time, basically no one has not issued any PT/YT. Even if someone mints some PT/YT, they can redeem them by burning together (redeemWithYt) with minimum issuance fee loss.

Escalate to make sponsor's disputation to be seen by Sherlock judge

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.

nevillehuang commented 4 months ago

The root cause is that scale can be manipulated, which is based on a kind of inflation attack. This issue can be prevented by minting some shares earlier than a attacker.

This is an assumption that should have been made known in the known risks of contest details.

ydspa commented 4 months ago

The root cause is that scale can be manipulated, which is based on a kind of inflation attack. This issue can be prevented by minting some shares earlier than a attacker.

This is an assumption that should have been made known in the known risks of contest details.

Same idea, even though this is not high due to some external assumptions , it's at least a medium.

xiaoming9090 commented 4 months ago

The issue should remain a valid High, as the mitigation in the escalation cannot be applied retrospectively after the contest.

sherlock-admin4 commented 4 months ago

The protocol team fixed this issue in PR/commit https://github.com/napierfi/napier-v1/pull/176.

cvetanovv commented 4 months ago

I disagree with the escalation and this report should remain a valid High. I agree with @nevillehuang comment that the sponsor should have left additional information in the readme as a known issue to consider the report invalid.

Czar102 commented 4 months ago

@massun-onibakuchi @ydspa @nevillehuang @xiaoming9090 @cvetanovv Could you elaborate on the size of the loss? I'm not sure if it's something of a loss of interest or not.

xiaoming9090 commented 4 months ago

@massun-onibakuchi @ydspa @nevillehuang @xiaoming9090 @cvetanovv Could you elaborate on the size of the loss? I'm not sure if it's something of a loss of interest or not.

@Czar102 This issue is not related to the loss of interest or yield. The YT (Yield Token) holders are entitled to two sources of income:

  1. Yield/Interest from the Yield Tokens they are holding on. This is dependent on the performance of the underlying interest-bearing assets (e.g., stETH).
  2. A portion of the principal (assets) allocated by the PT (Principle Token) holders that they will receive upon maturity if it is a sunny day. If the tilt ($\theta$) is set to 10%, around 10% of what the PT holders deposited into the protocol belongs to the YT holders.

This issue only concerns the second source of income and is not related to the first one. The loss is dependent on the amount of deposit to the protocol. If 1M worth of PT tokens are being issued/minted and the tilt is 10%, then YT holders would be expected to lose around 100,000 if the manipulation takes place.

ydspa commented 4 months ago

@massun-onibakuchi @ydspa @nevillehuang @xiaoming9090 @cvetanovv Could you elaborate on the size of the loss? I'm not sure if it's something of a loss of interest or not.

The loss looks like highly constrained, in current deployment script tilt is set to 0%, in test cases it is set to 0.1%. That is 1M fund in tranche causes 1k loss. reference:

File: test\unit\Tranche.t.sol
68:         _tilt = 10; // 1e4 = 100%

File: test\unit\TrancheFactory.t.sol
19:         _tilt = 10;

File: script\MockDeploy.s.sol
16:     uint256 tilt = 0;
xiaoming9090 commented 4 months ago

Even a 0.1% loss is sufficient to be classified as a substantial loss of assets. 1M leads to 1000 losses, while 10M leads to 10,000 losses, and so on. We could only classify a loss as insignificant when it is a dust amount. However, in this case, it is not a dust amount.

The MockDeploy.s.sol script is a mock file, as the name suggests, and does not represent the actual deployment.

Czar102 commented 4 months ago

Got it, @xiaoming9090 don't you think this issue constitutes Medium severity based on sponsor's comment?

Even if someone mints some PT/YT, they can redeem them by burning together (redeemWithYt) with minimum issuance fee loss.

xiaoming9090 commented 4 months ago

Got it, @xiaoming9090 don't you think this issue constitutes Medium severity based on sponsor's comment?

Even if someone mints some PT/YT, they can redeem them by burning together (redeemWithYt) with minimum issuance fee loss.

@Czar102

Let PT be the Principal Token and YT be the Yield Token. The Principal Token allows one to claim the principal amount of the assets after maturity, while the Yield Token allows one to claim the yield from the interest-bearing assets.

The sponsor states a scenario in which Alice deposits some underlying assets and mints/issues 100 PT and 100 YT. She will hold 100 PT and 100 YT (equal amount) and be both holders of PT and YT simultaneously.

Let the portion of the principal (assets) allocated by the PT holders that the YT holder is entitled to receive be $X$. The issue in this report is that the YT holders will not receive $X$ due to manipulation, and the PT holders will keep $X$ for themselves. Since Alice is both the PT and YT holders, she will be "delta neutral" as her loss is covered by her gain.

However, the report concerns a more realistic use case of the protocol in which Alice is only a PT holder and Bob is only a YT holder. Note that this is a Yield Stripping protocol, which splits interest-bearing assets into Principal Tokens and Yield Tokens. So, in the real world, most users/traders would have their trade strategy to either only hold PT tokens to get a fixed rate yield to lock the gain (Fixed-Yield Strategy) OR only hold YT tokens to long yield (Profit when APY goes up)(Long-Yield Strategy).

If one holds equal amounts of PT and YT in their portfolio, it is equivalent to holding interest-bearing assets, and there is no need for Yield Stripping in the first place.

In this use case, Alice is only a PT holder, and Bob is only a YT holder. Alice's manipulation will allow her to keep her portion of the principal, but it leads to Bob's loss as he cannot receive the portion of the principal he is entitled to.

Czar102 commented 4 months ago

What I meant is that the scale and max scale should be checked by the buyer as a parameter of the position they are purchasing. So, YT holders shouldn't be expecting a sunny day payout for these parameters.

Am I misunderstanding anything? @xiaoming9090

ydspa commented 4 months ago

Even a 0.1% loss is sufficient to be classified as a substantial loss of assets. 1M leads to 1000 losses, while 10M leads to 10,000 losses, and so on. We could only classify a loss as insignificant when it is a dust amount. However, in this case, it is not a dust amount.

The MockDeploy.s.sol script is a mock file, as the name suggests, and does not represent the actual deployment.

Two more thing should be pointed out, let's say there is 10M PT (worth 10M USD) in the Tranche: 1) It doesn't mean the over all loss is 0.1% worth of 10K USD, if there are 8M PT + YT holders, 2M PT only holders, and 2M YT only holders, then the over all loss from YT only holders is 2M * 0.1% = 2K USD. 1) It also doesn't mean the attacker self can get all the 2K USD profit, if the attacker only owns 10K PT, he/she can only get 10 USD profit.

IMO, it's a grief attack benefiting the attacker not so much and causing very limited over all loss.

xiaoming9090 commented 4 months ago

What I meant is that the scale and max scale should be checked by the buyer as a parameter of the position they are purchasing. So, YT holders shouldn't be expecting a sunny day payout for these parameters.

Am I misunderstanding anything? @xiaoming9090

Technically, they can fetch the scale and max scales by polling the on-chain contracts to determine if a Tranch is a Sunny Day based on the formula ($\frac{scale}{max scales} >= 1 - \theta$) if they are aware of the risk of the max scale being manipulated in the first place, but I doubt normal retail users would do so.

It is unsure if the UI would include information regarding whether the Tranche is sunny or not, with drill-down details of the current scale and max scales. Even if the UI states clearly that the Tranche is not sunny, it does not "scare" away all traders because some traders who want to long yield will still obtain the Yield Token (YT) regardless of whether they will receive a portion of the principal allocated by PT holders or not after maturity. This group of traders is mainly interested in collecting the yield from the underlying interest-bearing assets (e.g., stETH). However, this does not translate that it's fine for this group of traders to lose their portion of the principal allocated by PT holders.

On the other hand, it might be possible that some traders are eyeing receiving a portion of the principal allocated after maturity if the Tranche ends up being on a sunny day.

xiaoming9090 commented 4 months ago

Even a 0.1% loss is sufficient to be classified as a substantial loss of assets. 1M leads to 1000 losses, while 10M leads to 10,000 losses, and so on. We could only classify a loss as insignificant when it is a dust amount. However, in this case, it is not a dust amount. The MockDeploy.s.sol script is a mock file, as the name suggests, and does not represent the actual deployment.

Two more thing should be pointed out, let's say there is 10M PT (worth 10M USD) in the Tranche:

  1. It doesn't mean the over all loss is 0.1% worth of 10K USD, if there are 8M PT + YT holders, 2M PT only holders, and 2M YT only holders, then the over all loss from YT only holders is 2M * 0.1% = 2K USD.
  2. It also doesn't mean the attacker self can get all the 2K USD profit, if the attacker only owns 10K PT, he/she can only get 10 USD profit.

IMO, it's a grief attack benefiting the attacker not so much and causing very limited over all loss.

If there are 8M PT + YT tokens in circulation, a more likely scenario would be 8M PT only holders and 8M YT only holders, or close to that.

Most users/traders would have their trade strategy to either only hold PT tokens to get a fixed rate yield to lock the gain (Fixed-Yield Strategy) OR only hold YT tokens to long yield (Profit when APY goes up)(Long-Yield Strategy).

If one holds equal amounts of PT and YT in their portfolio, it is equivalent to holding interest-bearing assets, and there is no need for Yield Stripping in the first place.

Czar102 commented 4 months ago

It is unsure if the UI would include information regarding whether the Tranche is sunny or not, with drill-down details of the current scale and max scales.

I think the scale/maxscale is a parameter of the setup token bought, and users should be aware of it and price the token accordingly. Would also like to get to know @massun-onibakuchi's opinion.

I think this is Medium severity at most. @xiaoming9090 would you agree, based on the above?

xiaoming9090 commented 4 months ago

@Czar102 In my opinion, I believe it is unlikely that the UI would include information related to whether the Tranche is sunny or not, with drill-down details of the current scale and max scales that the users could utilize to price the tokens themselves. Since the UI was not shared during the audit period, and this issue has already been remediated by removing the tilt/sunny mechanism entirely, there is no definitive answer regarding what information will be shared in the UI in the final form of the protocol.

The YT tokens are minted/issued based only on the current exchange rate (scale). Thus, even with the information on the scale and max scale (the max scale is the variable being manipulated) represented in the table, most retail users would not take note of these variables and would not effectively use this information to perform additional calculations to derive the real value of the YT tokens after considering the current max scale.

However, given this uncertainty, I could agree with your view in this case, and a High -> Medium would be fair.

Czar102 commented 4 months ago

Result: Medium Has duplicates

Lack of users' understanding of the protocol is not an issue. But, this issue would cause the system to work not as expected, which effectively causes loss of a part of core functionality. Hence, considering this issue a valid Medium.

sherlock-admin4 commented 4 months ago

Escalations have been resolved successfully!

Escalation status:

Czar102 commented 4 months ago

This is a High severity issue as a duplicate of a High severity issue.

sherlock-admin4 commented 4 months ago

The Lead Senior Watson signed off on the fix.