The user's yield is updated in beforeYtTransfer. This means yield will not be updated if only PT is transferred.

[c4-bot-9]

Lines of code

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

Vulnerability details

Impact

User's yield will not be updated if they transfer PT tokens without also transferring YT tokens. This means that if a user earns yield through their YT tokens, and then transfers PT tokens, their yield will not be incorporated into the Principal Token contract until they also transfer YT tokens. For users who only interact with PT tokens, this could lead to lost or inaccurate yield tracking. Their earned yield would not be updated or claimable until they also transfer YTs. This violates the expected behavior and is a high severity issue.

Proof of Concept

The beforeYtTransfer function is only called when a YT transfer happens, not when a PT transfer happens. This means that user yield is only updated when YTs are transferred, but not when only PTs are transferred. Here is the relevant code:

  function beforeYtTransfer(address _from, address _to) external override {
       if (msg.sender != yt) {
           revert UnauthorizedCaller(); 
       }
       updateYield(_from);
       updateYield(_to);
    }

This shows that updateYield is only called for the _from and _to addresses when a YT transfer happens (since only the YT contract can call this function). The impact is that if a user transfers PTs, their yield will not be updated. This means the yield calculations could be incorrect if the user later redeems or transfers YTs, since the rates would not have been updated. For example:

1. Alice deposits and has 100 PTs and 100 YTs
2. The yield rate decreases
3. Alice transfers 50 PTs to Bob
4. Alice's yield is not updated since only PTs were transferred
5. Later Alice tries to redeem 50 YTs when the rate is much lower
6. But her yield will be calculated based on the old higher rate, overstating her actual yield This could allow users to manipulate yield calculations if they transfer only PTs.

Tools Used

Manual

Recommended Mitigation Steps

The updateYield function could be called on all PT transfers in the ERC20 _beforeTokenTransfer hook:

   function _beforeTokenTransfer(address _from, address _to, uint256 amount) 
   internal override {
     updateYield(_from); 
     updateYield(_to);
   }

This would ensure yields are updated on any token transfer, preventing manipulation.

Assessed type

Other

[c4-pre-sort]

gzeon-c4 marked the issue as duplicate of #109

[c4-pre-sort]

gzeon-c4 marked the issue as sufficient quality report

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid

[c4-judge]

JustDravee marked the issue as unsatisfactory: Invalid