When working with arithmetic operations in Solidity, it is very difficult to avoid potential inaccuracies due to precision loss. These sometimes small discrepancies might lead to HUGE miscalculations if not handled properly. Precision loss and rounding errors could lead to loss of funds for both protocols and users alike. Luckily there are some ways to mitigate them and we'll be discussing them in this article.

I've incorporated examples of two problems I identified (credits to 33BYTEZZZ) for a clearer understanding of these issues and the countermeasures to prevent such attacks.

Precision loss allows users to giveLoans to pools with less collateral than required

 interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;

        fees = (lenderFee * interest) / 10000;
        if(interest == 0 || fees == 0) revert PrecisionLoss();
        interest -= fees;

The _calculateInterest function is vulnerable to precision loss. Due to dividing twice the function will return zero for both interest and protocolInterest. This could lead to a lender giving their loan to another pool that doesn't have the balance to cover it. Leading to a loss for them and a gain for the future pools as they will have debts greater than their balance.

Vulnerability Details

giveLoan() calls _calculateInterest() which then returns the amount of interest the protocol fee and adds that to the loan (debt) in order to calculate the totalDebt.However, we already determined that these values are vulnerable to precision loss leading them to return 0 in some cases or be smaller than expected. That would lead to an inaccurate totalDebt and loanRatio allowing us to give a loan to a pool that has a higher loan ratio than our current pool which is not the expected behavior of the protocol.

The effect of this is the loanRatio value will be lower than expected.

            // assume the following
            // calculate intereset retunes the values totalDebt & loan.collateral 
            // totalDebt  = 30 and loan.collateral = 10
            // expected calculation  30 / 10 = 3
            // actual due to precision loss in _calculateInterest 20 / 10 = 2
            // maxLoanRatio = 2
            uint256 loanRatio = (totalDebt * 10 ** 18) / loan.collateral;
            // we would expect the next line to revert but
            // this would pass allowing us to create a loan with less collateral 
            // than needed for this pool
            if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();

This would allow a user to move their loan to a pool in which their loanRatio is greater than the maxloanRatio for that pool. Allowing them to move and create loans to pools with less collateral than the pool requires.

Solution

Verifying that neither the interest rate nor the fees are zero can help prevent precision loss and trigger a reversal if necessary. Additionally, establishing a minimum loan size could ensure that the left side of the equation consistently exceeds the right side.

 interest = (l.interestRate * l.debt * timeElapsed) / 10000 / 365 days;

        fees = (lenderFee * interest) / 10000;
        if(interest == 0 || fees == 0) revert PrecisionLoss();
        interest -= fees;

Another solution is creating a formula that always rounds up. This is in favor of the lender and the protocol as well. Something like this would suffice.

    /**
     * @param a numerator
     * @param b denominator
     * @dev Division, rounded up
     */
    function roundUpDiv(uint256 a, uint256 b) internal pure returns (uint256) {
        if (a == 0) return 0;
        return (a - 1) / b + 1;
    }

Potential Distribution Failure Due to Zero Rounded Transfer Amount

The identified vulnerability is associated with the _distribute function in
Distributor.sol. In scenarios where the total token amount is low or low values are used for percentages, the function may encounter a precision issue.
This arises due to the division of totalAmount by BASIS_POINTS to calculate the distribution amount for each winner. The precision error can lead to incorrect token distribution, affecting the fairness and accuracy of rewards to winners.

        for (uint256 i; i < winnersLength;) {
            uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
            erc20.safeTransfer(winners[i], amount);
            unchecked {
                ++i;
            }
        }

The vulnerability stems from the calculation of amount within the distribution loop. The formula amount = totalAmount * percentages[i] / BASIS_POINTS involves a division operation that could result in loss of precision(Rounding to Zero) when dealing with small totalAmount values or low percentages[i]. This imprecision can lead to token amounts being rounded down to zero, resulting in unfair or incomplete rewards for winners.

Proof Of Concept:

To simulate the vulnerability we need to make changes in the modifier
setUpContestForJasonAndSentJpycv2Token:

Code:

    modifier setUpContestForJasonAndSentJpycv2Token(address _organizer) {
        vm.startPrank(factoryAdmin);
        bytes32 randomId = keccak256(abi.encode("Jason", "001"));
        proxyFactory.setContest(_organizer, randomId, block.timestamp + 8 days, address(distributor));
        vm.stopPrank();
        bytes32 salt = keccak256(abi.encode(_organizer, randomId, address(distributor)));
        address proxyAddress = proxyFactory.getProxyAddress(salt, address(distributor));
        vm.startPrank(sponsor);
        MockERC20(jpycv2Address).transfer(proxyAddress, 10);
        vm.stopPrank();
        // console.log(MockERC20(jpycv2Address).balanceOf(proxyAddress));
        // assertEq(MockERC20(jpycv2Address).balanceOf(proxyAddress), 10000 ether);
        _;
    }

We change the value transferred to the proxyAddress to 10 tokens.

Note: We are not using 10 ether here as ether has 18 decimals which misguides the intended attack.

Now, we create a function called testPrecisionLoss() wherein we simulate the end of a contest and call the deployProxyAndDistribute() function. This makes use of the modified createData() function to send in 95 winners, each being rewarded with a percentage of 100 BASIS POINTS, which is equal to (10000 - COMMISSION_FEE) i.e. 9500 BASIS POINTS thus satisfying the conditions in the _distribute() function and allowing distribution of funds.

Code:

function testPrecisionLoss() public setUpContestForJasonAndSentJpycv2Token(organizer) {
        bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));

        //create data from modified createData() function
                bytes memory data = createData();

        vm.startPrank(organizer);
        console.log("User1 Start Balance -", MockERC20(jpycv2Address).balanceOf(user1));
        console.log("Stadium Balance Before: ",MockERC20(jpycv2Address).balanceOf(stadiumAddress));

        //warping to the time where contest ends and token distribution is allowed
                vm.warp(30 days);

                // distributing the rewards to all 95 winners 
        proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), data);

        console.log("Stadium End Balance: ",MockERC20(jpycv2Address).balanceOf(stadiumAddress));
        console.log("User1 After Balance -", MockERC20(jpycv2Address).balanceOf(user1));

        vm.stopPrank();
    }

The logs prove the existence of precision loss:

Running 1 test for test/integration/ProxyFactoryTest.t.sol:ProxyFactoryTest
[PASS] testPrecisionLoss() (gas: 892788)
Logs:
  0x000000000000000000000000000000000000000E
  User1 Start Balance - 0
  Stadium Balance Before:  0
  Stadium End Balance:  10
  User1 After Balance - 0

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.16ms

As we can see, all of the balance gets transferred to the stadiumAddress from the contract as the _commissionTransfer() function declares:

function _commissionTransfer(IERC20 token) internal {
        token.safeTransfer(STADIUM_ADDRESS, token.balanceOf(address(this)));
    }

Due to the precision loss(Rounding to Zero), none of the amount gets transferred to any winners and thus, all the "remaining tokens" get sent to the stadiumAddress, thus leaving our winners - in this case, user1 - rewarded with 0 balance.

The Rounding to Zero vulnerability has the potential to undermine the intended fairness and accuracy of the reward distribution process. In scenarios where the token balance is very small or percentages are low, the distribution algorithm could yield incorrect or negligible rewards to winners. This impacts the trust and credibility of the protocol, potentially leading to user dissatisfaction and decreased participation.

amount = totalAmount * percentages[i] / BASIS_POINTS;
       = 999 * 10 / 10000;
       = 0.999  = 0
// even amounts up to 999 WEI in GUSD or $.99 would cause precision loss. 
// which is significant loss to the protocol and the users themselves.

Recommendations

To mitigate against this type of attack predefined minimum threshold for the amount used in the token distribution calculation. This approach ensures that the calculated distribution amount remains above one so it won't round down.

Additionally, an alternative strategy involves adopting a rounding up equation that consistently rounds the distribution amount upward to the nearest integer value.

By incorporating either of these methodologies, the precision vulnerability associated with small values and percentages can be effectively mitigated.

Rounding Errors For Auditors