Incorrect negative yield calculation in `_computeYield`

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/PrincipalTokenUtil.sol#L98-L112

Vulnerability details

Description

_computeYield method calculates the negative yield amount incorrectly in certain edge cases.

When _oldIBTRate > _ibtRate (negative yield scenario), the logic computes yieldInAssetRay as the difference between the actualNegativeYieldInAssetRay and expectedNegativeYieldInAssetRay.

However, if the actual negative yield amount is larger than expected, this difference will be negative, representing a positive yield amount. This allows attackers to manipulate the parameters to extract extra yield.

The negative yield calculation subtracts the expected from the actual instead of the other way around.

Impact

An arbitrary positive yield can be generated by manipulating input parameters.

Proof of Concept

This vulnerability could allow attackers to manipulate the negative yield calculation to generate arbitrary positive yield amounts. They can exploit this to extract additional yield tokens from the protocol beyond what they are entitled to.

When _oldIBTRate > _ibtRate, _computeYield calculates the yield as the difference between actualNegativeYieldInAssetRay and expectedNegativeYieldInAssetRay. However, if the actual negative yield is larger than expected, their difference will be negative - representing a positive yield amount.

An attacker can exploit this by:

1. Depositing a small amount into the protocol to mint some PT and YT shares.

2. Manipulating the IBT rate via an oracle or flash loan to make _oldIBTRate much higher than _ibtRate.

3. When _computeYield is called, actualNegativeYieldInAssetRay will be large based on the high _oldIBTRate. But expectedNegativeYieldInAssetRay will be small based on the current low _ibtRate.

4. Their difference will be positive, allowing the attacker to extract extra positive yield.

File: src/libraries/PrincipalTokenUtil.sol::L98-L112

uint256 actualNegativeYieldInAssetRay = _convertToAssetsWithRate( <@ // based on high _oldIBTRate
    userYTBalanceInRay,
    _oldPTRate - _ptRate,
    RayMath.RAY_UNIT,
    Math.Rounding.Floor
);
uint256 expectedNegativeYieldInAssetRay = Math.ceilDiv(           <@// based on low _ibtRate 
    ibtOfPTInRay * (_oldIBTRate - _ibtRate),
    RayMath.RAY_UNIT
);
yieldInAssetRay = expectedNegativeYieldInAssetRay >
    actualNegativeYieldInAssetRay
    ? 0
    : actualNegativeYieldInAssetRay - expectedNegativeYieldInAssetRay;
yieldInAssetRay = yieldInAssetRay.fromRay(

By making actualNegativeYieldInAssetRay much larger than expectedNegativeYieldInAssetRay, their difference becomes positive.

Summary

The negative yield calculation in _computeYield incorrectly subtracts a higher expectedNegativeYield from a lower actualNegativeYield, allowing attackers to generate positive yield. This could lead to loss of user funds. The impact could be made more severe by manipulating _oldIBTRate further or calling _computeYield repeatedly.

Tools Used

Vs

Recommended Mitigation Steps

When the actual negative yield exceeds expected, the yield should be capped at 0.

Assessed type

Math

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-pre-sort]

gzeon-c4 marked the issue as primary issue

[c4-sponsor]

yanisepfl (sponsor) disputed

[yanisepfl]

What is detailed here works exactly as intended.

Let us imagine the simple scenario where the IBT rate goes from 1 to 0.5 to 0.75 and that there are user interactions with our protocol at those three steps, so that our protocol takes them into account. Therefore, the PT rate would start at 1 and then drop to 0.5 when the same drops happen in the IBT rate, and would not change upon IBT rate increase from 0.5 to 0.75. Hence, in our contract, for users that interacted with our protocol when the IBT rate was at 1, and then when it was at 0.75, the actual loss here would be considered as 0.5 asset per IBT (old PT rate - current PT rate), and the expected loss would be considered to be 0.25 asset per IBT (old IBT rate - current IBT rate). Therefore, per IBT generating yield for that user, we would consider that 0.5 - 0.25 = 0.25 assets per IBT will be assigned to that user.

Hence, the above example shows that the user gets assigned a positive yield in the described situation as per our protocol's design. If not, then the user would have suffer a loss of funds.

This issue is thus incorrect and we dispute it.

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid