Invariant Violation: Discrepancy in PT and YT Token Supplies

[c4-bot-6]

Lines of code

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

Vulnerability details

Summary

In the _withdrawShares and _beforeRedeem functions of the PrincipalToken contract, the burning process fails to properly account for both PT and YT tokens, leading to unequal total supplies.

Vulnerability Details

The code fails to maintain the invariant where the Principal Token (PT) and its associated Yield Token (YT) should always have equal supplies. Despite attempts to adjust the YT balance to 0 after expiration, incorrect burning functions prevent equalization.

invariant : https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/README.md?plain=1#L142

 //function _withdrawShares

       // burn owner's shares (YT and PT)
        if (block.timestamp < expiry) {
            IYieldToken(yt).burnWithoutUpdate(_owner, shares);
        }
        _burn(_owner, shares);
...

//function _beforeRedeem

        } else {
            updateYield(_owner);
            IYieldToken(yt).burnWithoutUpdate(_owner, _shares);
        }
        _burn(_owner, _shares);
...

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

Impact

Violation of the invariant results in discrepancies in PT and YT supplies. Inaccurate supply figures may cause confusion and inconsistency in token-related operations. Users and integrators relying on accurate supply information may face challenges.

Proof of Concept

1. withdraw Deposit
   • The user deposits 100 Usdc into the contract by calling the PrincipalToken::deposit function.
   • The PrincipalToken::_depositIBT function mint 100 PT and 100 YT tokens to the user's address.
   • Finally, Total supply of PT and YT increses by 100 withdraw
   • User calls PrincipalToken::withdraw function after maturity
   • withdraw function will call PrincipalToken::_withdrawShares
   • This line will not be called IYieldToken(yt).burnWithoutUpdate(_owner, shares); and YT total supply remains unchanged but PT total supply will be decrease

    function testWithdraw() public {
        uint256 amount = 1e18;
        underlying.approve(address(principalToken), amount);
        principalToken.deposit(amount, testUser);

        uint256 beforeBalance = underlying.balanceOf(testUser);
        assertEq(yt.totalSupply(), principalToken.totalSupply());

        _increaseTimeToExpiry();

        principalToken.withdraw(amount, testUser, testUser);

        assertEq(yt.totalSupply(), principalToken.totalSupply());

    }

2. Redeem Deposit
   • The user deposits 100 USDC into the contract by calling the PrincipalToken::deposit function.
   • The PrincipalToken::_depositIBT function mint 100 PT and 100 YT tokens to the user's address.
   • Therefore the total supply of PT and YT increses by 100 and still equall Redeem
   • user call PrincipalToken::redeem function after maturity
   • Redeem function will call PrincipalToken::_beforeRedeem
   • this line will not be called "IYieldToken(yt).burnWithoutUpdate(_owner, _shares);" and YT total supply remains unchanged
   • but PT total supply will be decrease
   • Finally, YT total supply will be greater than PT total supply.

    function testRedeem() public {
        uint256 amountToDeposit = 1e18;
        uint256 redeemShares = amountToDeposit / 2;
        vm.prank(testUser);

        underlying.approve(address(principalToken), amountToDeposit);
        principalToken.deposit(amountToDeposit, testUser);

        assertEq(yt.totalSupply(), principalToken.totalSupply());

        _increaseTimeToExpiry();

        principalToken.redeem(redeemShares, MOCK_ADDR_1, testUser);
        //assertEq(yt.totalSupply(), principalToken.totalSupply());
            assertGt(
                yt.totalSupply(),
                principalToken.totalSupply()
            );

    }

Tool used

Manual Review

Recommendation

Correct the burning functions _withdrawShares and _beforeRedeem to ensure proper burning of both PT and YT tokens, maintaining consistency in total supply. Implement checks to uphold the invariant where PT and YT should always have equal supplies.

Assessed type

Invalid Validation

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #114

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid