code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-claimer/blob/57a381aef690a27c9198f4340747155a71cae753/src/Claimer.sol#L60-L83

Vulnerability details

Impact

There is no validation of the length of winners and prizeIndices array in the claimPrizes function. If the length of the winners is lower than the prizeIndices, the rest of the prizeIndices will be skipped, so it's needed to check if it's the same length.

Proof of Concept

File: claimer/src/Claimer.sol#L60-L83
    function claimPrizes(
        Vault vault,
        uint8 tier,
        address[] calldata winners,
        uint32[][] calldata prizeIndices,
        address _feeRecipient
    ) external returns (uint256 totalFees) { 
        uint256 claimCount;
        for (uint i = 0; i < winners.length; i++) { // @audit here
            claimCount += prizeIndices[i].length;
        }

        uint96 feePerClaim = uint96(
        _computeFeePerClaim(
            _computeMaxFee(tier, prizePool.numberOfTiers()),
            claimCount,
            prizePool.claimCount()
        )
        );

        vault.claimPrizes(tier, winners, prizeIndices, feePerClaim, _feeRecipient);

        return feePerClaim * claimCount;
    }

Tools Used

Manual review

Recommended Mitigation Steps

Recommended adding the validation step to verify it has the same length.

File: claimer/src/Claimer.sol#L60-L83
    function claimPrizes(
        Vault vault,
        uint8 tier,
        address[] calldata winners,
        uint32[][] calldata prizeIndices,
        address _feeRecipient
    ) external returns (uint256 totalFees) { 
        require(winners.length == prizeIndices.length, "The length error"); // @audit here
        uint256 claimCount;
        for (uint i = 0; i < winners.length; i++) {
            claimCount += prizeIndices[i].length;
        }

        uint96 feePerClaim = uint96(
        _computeFeePerClaim(
            _computeMaxFee(tier, prizePool.numberOfTiers()),
            claimCount,
            prizePool.claimCount()
        )
        );

        vault.claimPrizes(tier, winners, prizeIndices, feePerClaim, _feeRecipient);

        return feePerClaim * claimCount;
    }

Assessed type

Invalid Validation

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)