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

4 stars 2 forks source link

Protocol Fails to Impose Tokenization Fee on Minimal Deposit Amounts #49

Closed c4-bot-1 closed 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

When a user attempts to deposit small sums, specifically an amount as low as 1, the tokenizationFee may be calculated as 0 due to the way fees are computed. This could potentially lead to unintended consequences where users are not charged the expected fees for tokenization, affecting the economic mechanisms of the protocol.

Proof of Concept

The issue arises in the following lines of the PrincipalToken.sol contract, where the tokenizationFee is computed based on the deposit amount:

uint256 tokenizationFee = PrincipalTokenUtil._computeTokenizationFee(
    _ibts,
    address(this),
    registry
);

link to code The internal function _computeTokenizationFee from PrincipalTokenUtil.sol is responsible for calculating the fee:

function _computeTokenizationFee(
    uint256 _amount,
    address _pt,
    address _registry
) internal view returns (uint256) { //@audit not check if returned fees are 0
    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
            );
}

link to internal function The issue is that if _amount is 1 and IRegistry(_registry).getTokenizationFee() is less than FEE_DIVISOR (1e18), the computed fee will be 0, which is not intended. This behavior occurs before the updateYield function call, suggesting a need for a pre-condition check to ensure tokenizationFee is not zero.

Tools Used

Manual

Recommended Mitigation Steps

It's recommended to add a validation check before updating the yield to ensure that the tokenizationFee is not zero. This can be done by inserting a require statement to check that tokenizationFee != 0, ensuring that all tokenizations are subject to a fee and maintaining the economic integrity of the protocol.

For instance, before the updateYield function call in PrincipalToken.sol:

require(tokenizationFee != 0, "Tokenization fee cannot be zero");

Assessed type

Math

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as insufficient quality report

gzeon-c4 commented 8 months ago

likely intended

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as duplicate of #124

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid