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

4 stars 2 forks source link

Users may receive yield for rate changes that occur after the expiry. #108

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/tokens/PrincipalToken.sol#L340

Vulnerability details

Summary

In the protocol, holders of YT are supposed to receive yield until the expiry. However, due to the way the protocol updates ibtRate and ptRate, users may be able to receive yield even if the rate changes occurred after the expiry.

Vulnerability Details

The protocol relies on user interactions to update ibtRate and ptRate. Specifically, these rates are updated only when PrincipalToken::updateYield is called during YT transfer, deposit, redeem, withdraw, or yield claim. In a hypothetical scenario where no user interacts at the expiry and the ibtRate of IBT changes afterward, users can receive yield that they are not supposed to accrue.

Impact

The impact of this vulnerability is MEDIUM. The rates at expiry are critical parameters in the protocol, but their correctness depends on user activities rather than a deterministic mechanism. This may allow users to receive unintended yield. Additionally, while I've only mentioned yield here, this vulnerability also applies to the assets that a user could redeem (e.g., if the rate decreased, the user could receive a reduced amount of assets).

Proof of Concept

The following test code can be added to ContractPrincipalToken1. The code verifies that a user can receive yield provided that no one has yet interacted with the protocol since the maturity date and the rate has changed.

    function testClaimYieldPostExpiry() public {
        uint256 amount = 1e18; 

        underlying.mint(MOCK_ADDR_1, amount);

        // MOCK_ADDR_1 deposits assets to PT
        vm.startPrank(MOCK_ADDR_1);
        underlying.approve(address(principalToken), amount);
        principalToken.deposit(amount, MOCK_ADDR_1);
        vm.stopPrank();

        // Timelapse past the expiry
        vm.warp(block.timestamp + principalToken.maturity() + 100);

        // Rate changes "after" the expiry
        _increaseRate(100);

        vm.prank(MOCK_ADDR_1);
        uint256 yieldClaimedADDR_1 = principalToken.claimYield(MOCK_ADDR_1);

        // Got positive yield even though the rate was changed "after" the expiry
        assertEq(yieldClaimedADDR_1, amount);      
    }

Tools Used

Foundry

Recommended Mitigation Steps

We could develop a dApp that triggers rate updates for the PT contract or implement another contract that maintains the expiry rates of all PT contracts used by the ecosystem.

Assessed type

Invalid Validation

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #103

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid