howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntropyGenerator/EntropyGenerator.sol#L168 https://github.com/code-423n4/2024-07-traitforge/blob/main/contracts/EntropyGenerator/EntropyGenerator.sol#L102

Vulnerability details

Impact

Inside of EntropyGenerator smart contract, there are two functions that are used to get entopies: getEntropy() and getNextEntropy(). The problem is that they check for incorrect max slot index at the start of the function which may result in an unexpected behavior.

Proof of Concept

There are only 770 slots designed to store 78 digit numbers (with 13 concatenated entropies in them). And the first slot has always index 0. That basically means that the last slot will have the index of 769. But in the current implementation of EntropyGenerator, it checks for 770 instead of 769:

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

  function getNextEntropy() public onlyAllowedCaller returns (uint256) {
    require(currentSlotIndex <= maxSlotIndex, 'Max slot index reached.');

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

  function getEntropy(
    uint256 slotIndex,
    uint256 numberIndex
  ) private view returns (uint256) {
    require(slotIndex <= maxSlotIndex, 'Slot index out of bounds.');

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

uint256 private maxSlotIndex = 770;

This check basically allows to get entropies for more slots than there are in existence and it can potentially result in an unexpected behavior and DoS.

Tools Used

Manual review.

Recommended Mitigation Steps

Replace the check above on this:

 require(currentSlotIndex < maxSlotIndex, 'Max slot index reached.');

Assessed type

Other

c4-judge commented 2 months ago

koolexcrypto changed the severity to QA (Quality Assurance)