Yield token holders can drain value from principal token holders

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

Yield token holders can drain value from principal token holders by manipulating ibt token exchange rate.

Proof of Concept

Exchange rate of ERC-4626 can be manipulated and it can be done for profit by yt token holders.

On each yt token balance change ibtRate and ptRate are updated. ptRate can only decrease and it happens in cases if currect exchange rate of ERC-4626 lower than stored rate, e.i. yield is negative. This condition can easily be triggered by anyone, naturally or on purpose.

After that principal token amount returned after maturity reduced permanently for principal token holders independent from future ibt rates.

PT token holders' ibt tokens doesn't disappear completely, they goes to yt token holders. So there is incentives for them to manipulate ibt exchange rate to theft principal from pt token holders.

// test/PrincipalToken/PrincipalToken.t.sol
function test_poc1() public {
    address alice = makeAddr("alice");

    // user deposits ibt tokens and sells yield to alice
    uint256 amountToDeposit = 1000e18;
    underlying.approve(address(principalToken), amountToDeposit);
    principalToken.deposit(amountToDeposit, address(this), alice);

    // alice manipulates exchange rate locally
    ibt.setPricePerFullShare(0.95e18);
    principalToken.updateYield(address(this));

    // ...but in the big picture everything is going well
    ibt.setPricePerFullShare(1.1e18);
    principalToken.updateYield(address(this));

    vm.warp(principalToken.maturity() + 1);

    // after maturity pt holder should get back his principal = 1000 tokens
    // but in reality he can only withdraw 950
    assertApproxEqAbs(
        principalToken.maxWithdraw(address(this)),
        950e18,
        1000
    );

    // for alice expected yield = 1000 * (1.1 - 10) = 100 underlying tokens
    // but in reality she stealed additional 50 from pt holder
    assertApproxEqAbs(
        principalToken.getCurrentYieldOfUserInIBT(alice) * 11/10, // convert IBT to underlying
        150e18,
        1000
    );
}

Tools Used

Manual review

Recommended Mitigation Steps

I can only recommend redesign of yield stripping system to handle cases when ibtRate can drop and then recover: pt holders shoudn't lose principal amount.

Assessed type

Math

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #305

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid