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

4 stars 2 forks source link

`PrincipalTokenUtil.sol` has rounding errors. #71

Closed c4-bot-7 closed 8 months ago

c4-bot-7 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L176-L185 https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L750-L769 https://github.com/code-423n4/2024-02-spectra/blob/main/src/libraries/PrincipalTokenUtil.sol#L157-L170

Vulnerability details

Impact

As the fee decreases, the yield to the protocol decreases. Although this value is small, if it is accumulated, the amount will become very large.

Proof of Concept

When user deposit underlying assets, he calls PricipalToken.sol#deposit function. The PricipalToken.sol#deposit function calls the PricipalToken.sol#_depositIBT function. Also the PricipalToken.sol#_depositIBT function calls PrincipalTokenUtil.sol#_computeTokenizationFee function. Rounding error occurs in the PrincipalTokenUtil.sol#_computeTokenizationFee function. PrincipalTokenUtil.sol#_computeTokenizationFee function is as follows.

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

And PricipalToken.sol#_depositIBT function is as follows.

    function _depositIBT(
        uint256 _ibts,
        address _ptReceiver,
        address _ytReceiver
    ) internal notExpired nonReentrant whenNotPaused returns (uint256 shares) {
        updateYield(_ytReceiver);
756:    uint256 tokenizationFee = PrincipalTokenUtil._computeTokenizationFee(
            _ibts,
            address(this),
            registry
        );
761:    _updateFees(tokenizationFee);
        shares = _convertIBTsToShares(_ibts - tokenizationFee, false);
        if (shares == 0) {
            revert RateError();
        }
        _mint(_ptReceiver, shares);
        emit Mint(msg.sender, _ptReceiver, shares);
        IYieldToken(yt).mint(_ytReceiver, shares);
    }

Also Pricipaltoken.sol#_updateFees function is as follows.

    function _updateFees(uint256 _feesInIBT) internal {
        unclaimedFeesInIBT += _feesInIBT;
        totalFeesInIBT += _feesInIBT;
    }

Finally PrincipalToken.sol#claimYield function uses accumulated uclaimedFeesInIBT and this protocol gains yield. PrincipalToken.sol#claimYield function is as follows.

    function claimYield(address _receiver) public override returns (uint256 yieldInAsset) {
        uint256 yieldInIBT = _claimYield();
        if (yieldInIBT != 0) {
            yieldInAsset = IERC4626(ibt).redeem(yieldInIBT, _receiver, address(this));
        }
    }

Therefore the smaller tokenizationFee, the smaller the yield to the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

PrincipalTokenUtil.sol#_computeTokenizationFee function is as follows.

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

This function Modifies as follows.

    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
-               );
+               .mulDiv(IRegistry(_registry).getTokenizationFee()*FEE_DIVISOR - IRegistry(_registry).getFeeReduction(_pt, msg.sender), 
+                   FEE_DIVISOR*FEE_DIVISOR, Math.Rounding.Ceil)
    }

Assessed type

Math

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 #27

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid