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

4 stars 2 forks source link

Fee reduction calls the wrong recipient #14

Closed c4-bot-9 closed 8 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/383202d0b84985122fe1ba53cfbbb68f18ba3986/src/libraries/PrincipalTokenUtil.sol#L163-L169

Vulnerability details

Impact

Users will not get a fee reduction when tokenizing their IBT tokens.

Proof of Concept

When a user deposits their ASSET token or IBT tokens, _depositIBT() will be called.

    function _depositIBT(
        uint256 _ibts,
        address _ptReceiver,
        address _ytReceiver
    ) internal notExpired nonReentrant whenNotPaused returns (uint256 shares) {
        updateYield(_ytReceiver);
>       uint256 tokenizationFee = PrincipalTokenUtil._computeTokenizationFee(
            _ibts,
            address(this),
            registry
        );

There is a fee for tokenizing their IBT tokens into PT/YT tokens. The fee is calculated through _computeTokenizationFee()

    function _computeTokenizationFee(
        uint256 _amount,
        address _pt,
        address _registry
    ) internal view returns (uint256) {
        return
            _amount
                .mulDiv(IRegistry(_registry).getTokenizationFee(), FEE_DIVISOR, Math.Rounding.Ceil)
                .mulDiv(
>                   FEE_DIVISOR - IRegistry(_registry).getFeeReduction(_pt, msg.sender),
                    FEE_DIVISOR,
                    Math.Rounding.Ceil
                );
    }

Note that msg.sender is used, which is incorrect since msg.sender here refers to the PrincipalToken contract, and not the actual caller of the deposit function. This is because the PrincipalToken contract is calling the function in the PrincipalTokenUtil contract, so the message sender changes to the PrincipalToken contract.

Even if the user is whitelisted to get a discount when tokenizing their IBT tokens, the discount will not be applicable.

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend adding a recipient parameter and passing the right msg.sender into the _computeTokenizationFee() function

    function _depositIBT(
        uint256 _ibts,
        address _ptReceiver,
        address _ytReceiver
    ) internal notExpired nonReentrant whenNotPaused returns (uint256 shares) {
        updateYield(_ytReceiver);
        uint256 tokenizationFee = PrincipalTokenUtil._computeTokenizationFee(
            _ibts,
            address(this),
            registry,
            //@audit: Here, msg.sender will be the depositor.
+           msg.sender
        );
function _computeTokenizationFee(
        uint256 _amount,
        address _pt,
        address _registry,
+       address recipient
}
        ...
+                    FEE_DIVISOR - IRegistry(_registry).getFeeReduction(_pt, recipient),

Assessed type

Context

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #124

gzeon-c4 commented 8 months ago

msg.sender not changed in a internal library call

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid