Use of Unchecked Arithmetic in Percentage Calculation

Severity

Medium Risk

Summary

The _distribute function calculates the distribution amount using unchecked arithmetic, which can lead to integer overflow vulnerabilities if the sum of percentages and totalAmount exceeds the maximum value of a uint256. This could result in incorrect token distribution and potential loss of funds.

Vulnerability Details

The _distribute function calculates the amount to be distributed using the formula totalAmount * percentages[i] / BASIS_POINTS. If the sum of percentages and totalAmount is sufficiently large, an unchecked multiplication could result in integer overflow, causing the calculated amount to be incorrect.

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
    internal
{
    // ... (other code)

    uint256 totalAmount = erc20.balanceOf(address(this));

    // if there is no token to distribute, then revert
    if (totalAmount == 0) revert Distributor__NoTokenToDistribute();

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

    // ... (other code)
}

Impact

Integer overflow can lead to incorrect token distribution, where users might receive more tokens than intended or even negative amounts. This could result in financial losses and undermine the fairness of the distribution process.

Tools Used

Manual

Recommendations

To prevent integer overflow vulnerabilities, use safe arithmetic operations and validate input values before performing calculations. You can use the OpenZeppelin SafeMath library to ensure safe arithmetic operations:

import {SafeMath} from "openzeppelin/utils/math/SafeMath.sol";

// ...

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
    internal
{
    // ... (other code)

    uint256 totalAmount = erc20.balanceOf(address(this));

    // if there is no token to distribute, then revert
    if (totalAmount == 0) revert Distributor__NoTokenToDistribute();

    uint256 winnersLength = winners.length; // cache length
    for (uint256 i; i < winnersLength;) {
        // Use SafeMath to perform safe arithmetic operations
        uint256 amount = SafeMath.div(SafeMath.mul(totalAmount, percentages[i]), BASIS_POINTS);
        erc20.safeTransfer(winners[i], amount);
        unchecked {
            ++i;
        }
    }

    // ... (other code)
}

By using the SafeMath library, you ensure that arithmetic operations are performed safely, preventing integer overflow vulnerabilities.

Cyfrin / 2023-08-sparkn