sherlock-audit/2023-04-footium-judging

juancito - One extra academy player can be minted per season due to mischeck in `mintPlayers`

sherlock-admin opened this issue · 0 comments

juancito

medium

One extra academy player can be minted per season due to mischeck in mintPlayers

Summary

The limit of academy players that can be minted per season is given by _maxGenerationId.

Due to a mischeck it is possible to mint one extra player.

Vulnerability Detail

mintPlayers lets mint academy players from 0 up to _maxGenerationId inclusive:

    for (uint256 i = 0; i < generationIds.length; ) {
        generationId = generationIds[i];

        if (generationId > _maxGenerationId) { // @audit mischeck
            revert GenerationIDTooHigh(generationId, _maxGenerationId);
        }

Add this test to FootiumAcademy.test.ts to prove how it is possible to mint 11 players when the limit is 10:

    it.only("mints one player more than the maxGenerationId", async () => {
        const maxGenerationId = 10; // @audit max 10 players
        await expect(academy.changeMaxGenerationId(maxGenerationId))
            .to.emit(academy, "ChangedMaxGenerationId")
            .withArgs(maxGenerationId);

        //const token = [clubId = 7, division = 4]
        const clubDivProof = clubDivsTree.getHexProof(
            hashClubDivisionInputs([7, 4])
        );

        const divisionTier = 4;
        const generationIds = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; // @audit 11 players > maxGenerationId
        const ethAmount = ethers.BigNumber.from(divisionFees[0]).mul(
            generationIds.length
        );

        await expect(
            academy
                .connect(addr1)
                .mintPlayers(
                    1,
                    7,
                    divisionTier,
                    generationIds,
                    clubDivProof,
                    {
                        value: ethAmount,
                    }
                )
        ).to.not.reverted;
    });

Impact

An extra academy player can be minted per season bypassing the restriction set by _maxGenerationId

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumAcademy.sol#L186

Tool used

Manual Review

Recommendation

Prevent minting the last index, when generationId == _maxGenerationId:

-   if (generationId > _maxGenerationId) {
+   if (generationId >= _maxGenerationId) {
        revert GenerationIDTooHigh(generationId, _maxGenerationId);
    }

Duplicate of #152