code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

Non-compliance with ERC20 Standard in YT #16

Closed c4-bot-9 closed 8 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/perspectivefi/spectra-core/blob/main/src/tokens/YieldToken.sol#L120-L125

Vulnerability details

Impact

After the maturity of the PT date, the balanceOf function for any account return 0. In fact, tokens still exists: OpenZeppelin ERC20Upgradeable _balances variable is not necessary equal to 0 for all accounts. Moreover, it is still possible to transfer YT tokens without throwing by calling the transfer function for example.

Here is ERC20 transfer function standard citation:

The function SHOULD throw if the message caller’s account balance does not have enough tokens to spend.

Here is ERC20 balanceOf function standard citation:

Returns the account balance of another account with address _owner.

If we assume that address A has 10 tokens before maturity. After the PT's maturity date is reached, if the YT follows ERC20 standard, or the token balanceOf function should return 10, either transfer function should throw for the address A.

The issue rely on the fact that the price value of a token is different from the token itself. Even if tokens have no value, they still exist.

Tools Used

Manual review

Recommended Mitigation Steps

I recommend to check if block.timestamp < IPrincipalToken(pt).maturity() on every transfer. In that way, it will throw after PT's maturity date.

Assessed type

ERC20

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as primary issue

c4-sponsor commented 8 months ago

yanisepfl (sponsor) disputed

c4-sponsor commented 8 months ago

yanisepfl marked the issue as disagree with severity

yanisepfl commented 8 months ago

After maturity, YTs become useless and it does not matter if they do not strictly follow ERC20 anymore. In particular, at and after expiry, YTs have no value anymore and we do not bother what can be done with the contract, including any supply or balance consistency. Hence, users can do whatever they want with them after maturity and our protocol will behave as intended. Therefore, we disagree with the severity of this issue and consider this as a QA report. Thanks for the recommendation!

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid