code-423n4 / 2024-07-traitforge-findings

1 stars 0 forks source link

Forged entity sharing the accounting from `10,000` quota, would make the quota fill before for the normal mint Golden God slot index is reached for a generation #477

Closed howlbot-integration[bot] closed 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/TraitForgeNft/TraitForgeNft.sol#L328

Vulnerability details

Details

Impact

No Golden God will be selected for a generation due to quota getting filled by the count of forged entities and thus the entropy generator slot index for Golden God will not be reached, resulting in breaking of the invariant: 1 Golden God per generation.

Proof of Concept

  1. Suppose that total 10,000 entities are minted for Generation 1.
  2. User tries to forge with one another, and lets say that total 4000 entity are minted by forging to get to gen 2.
  3. Now, this means that total 6000 entity can be minted (non-forging NFTs).
  4. Let's say for gen 2, the Golden God is at any slot index.
  5. Now starting 512 slots doesn't have Golden God, which accounts for (512 * 13) = 6656 entities.
  6. But here as total of 6000 entity are possible to be minted (non-forged), thus the Golden God will never arrive, even if the Golden God was present at start then also it won't be reached, as 6656 entities were to be minted until Golden God, and the quota remaining for this example was of only 6000.
  7. Thus, Golden God will never be minted for this gen (gen 2 for this example), and thus violates the invariant: 1 Golden God per gen.

Tools Used

Manual Review

Recommended Mitigation Steps

Have a different quota for both forged and normal minted NFTs and also align the entropy generator slot in accordance with the normal minting NFT count.

Assessed type

Context

c4-judge commented 2 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 2 months ago

koolexcrypto marked the issue as duplicate of #656

c4-judge commented 1 month ago

koolexcrypto changed the severity to 2 (Med Risk)