Cyfrin / 2023-08-sparkn

Other
11 stars 15 forks source link

Contract can crashes / possible incorrect behavior due to Unchecked Array Access #878

Closed codehawks-bot closed 1 year ago

codehawks-bot commented 1 year ago

Contract can crashes / possible incorrect behavior due to Unchecked Array Access

Severity

Medium Risk

Summary

The code contains array accesses that might lead to out-of-bounds errors if not properly validated. This can result in contract crashes or incorrect behavior.

Vulnerability Details

The contract does not adequately validate array indices before accessing arrays. This lack of validation could allow attackers to manipulate array indices and potentially cause out-of-bounds errors, leading to contract crashes or unintended behavior.

// Vulnerable Code: Lack of Array Index Validation
function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data) public {
    require(msg.sender == address(proxyFactory), "Distributor__OnlyFactoryAddressIsAllowed");
    require(winners.length == percentages.length, "Distributor__MismatchedArrays");

    // ...

    for (uint256 i = 0; i < winners.length; i++) {
        // Vulnerable Code: Lack of Index Validation
        MockERC20(token).transfer(winners[i], (MockERC20(token).balanceOf(address(this)) * percentages[i]) / 10000);
    }

    emit Distributed(token, winners, percentages, data);
}

Impact

Unchecked array access can lead to out-of-bounds errors, causing contract crashes, unexpected behavior, and potential manipulation of contract state. Attackers could exploit this vulnerability to disrupt contract functionality and potentially exploit vulnerabilities.

Tools Used

Manual / Foundry

Recommendations

  1. Implement proper validation for array indices to ensure they are within valid ranges before accessing arrays.

  2. Utilize the require statement to validate array indices and lengths, preventing potential out-of-bounds errors.

By implementing these recommendations, you can safeguard against out-of-bounds errors and ensure the contract's array accesses are safe and reliable.

PatrickAlphaC commented 1 year ago

reward distribution inputs controlled by organizer or owner, attackers can't control these inputs