juancito - One extra academy player can be minted per season due to mischeck in `mintPlayers`
sherlock-admin opened this issue · 0 comments
sherlock-admin commented
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
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