sherlock-audit / 2024-08-flayer-judging

0 stars 0 forks source link

Unique Inky Puppy - Loss of Precision in `Taxcalculator::calculateProtectedInterest()` due to division before multiplication. #710

Open sherlock-admin3 opened 4 days ago

sherlock-admin3 commented 4 days ago

Unique Inky Puppy

Medium

Loss of Precision in Taxcalculator::calculateProtectedInterest() due to division before multiplication.

Summary

There's a possible loss of precision due to division before multiplication in Taxcalculator::calculateProtectedInterest() which can lead to miscalculation of the _interestRate()

Vulnerability Detail

There's a possible loss of precision due to division before multiplication in Taxcalculator::calculateProtectedInterest() which can lead to miscalculation of the _interestRate()

Vulnerability Detail

The function calculateProtectedInterest() is used to calculate the interest rate for the protected listings. But the issue here is that the method of calculating the interestRate when the KINK value has been exceeded is susceptible to precision loss due to division before multiplication which can generate wrong values for the CompoundedFactor.

Here is an illustration with simplified values to further express the point The present code for the derivation of interestRate_ when the KINK has been exceeded.

interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;

lets take for example that our _utlizatiomRate = 0.8 and UTILIZATION_INK = 0.5

interestRate_ = (((0.8 ether - 0.5 ether) * (100 - 8)) / (1 ether - 0.5 ether) + 8) * 100;
             = ((0.3 ether * 92) / 0.5 ether + 8) * 100;
             = (27.6 ether / 0.5 ether + 8) * 100; //here 27.6 / 0.5 will give 55.2 but solidity truncates this value to 55.
             = (55 + 8) * 100;
             = 6300.

But when properly corrected it can be seen to give a value with better precision.

interestRate_ = (((0.8 ether - 0.5 ether) * (100 - 8) * 100) / (1 ether - 0.5 ether) + 8) * 100;
             = ((0.3 ether * 92 * 100) / 0.5 ether) + 800;
             = (2760 ether / 0.5 ether) + 800;
             = 5520 + 800; //here the precision is preserved.
             = 6320.

Impact

This will lead to part of the interest which should have accrued on the protectedListings of users being lost, leading to loss of funds for the protocol as users pay less when unlocking their protected listing.

Code Snippet

 function calculateProtectedInterest(uint _utilizationRate) public pure returns (uint interestRate_) {
        // If we haven't reached our kink, then we can just return the base fee
        if (_utilizationRate <= UTILIZATION_KINK) {
            // Calculate percentage increase for input range 0 to 0.8 ether (2% to 8%)
            interestRate_ = 200 + (_utilizationRate * 600) / UTILIZATION_KINK;
        }
        // If we have passed our kink value, then we need to calculate our additional fee
        else {
            // Convert value in the range 0.8 to 1 to the respective percentage between 8% and
            // 100% and make it accurate to 2 decimal places.
   @>       interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;

Tool used

Manual Review

Recommendation

Rearrange the code by multiplying by 100 before performing the division, which scales up the numerator and preserves more precision during the division operation.

 function calculateProtectedInterest(uint _utilizationRate) public pure returns (uint interestRate_) {
        // If we haven't reached our kink, then we can just return the base fee
        if (_utilizationRate <= UTILIZATION_KINK) {
            // Calculate percentage increase for input range 0 to 0.8 ether (2% to 8%)
            interestRate_ = 200 + (_utilizationRate * 600) / UTILIZATION_KINK;
        }
        // If we have passed our kink value, then we need to calculate our additional fee
        else {
            // Convert value in the range 0.8 to 1 to the respective percentage between 8% and
            // 100% and make it accurate to 2 decimal places.
--          interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8)) / (1 ether - UTILIZATION_KINK) + 8) * 100;
++          interestRate_ = (((_utilizationRate - UTILIZATION_KINK) * (100 - 8) * 100) / (1 ether - UTILIZATION_KINK) + 8) * 100;

        }
    }

Here the precision is more preserved than in the initial method.