howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntropyGenerator/EntropyGenerator.sol#L181

Vulnerability details

Description of Issue

The EntropyGenerator contract contains a potential precision loss in the entropy calculation function. The current implementation uses integer division followed by a modulo operation, which can lead to biased results due to truncation errors.

The problematic code is:

uint256 entropy = (slotValue / (10 ** (72 - position))) % 1000000;

This calculation first performs an integer division, which can lose precision, especially for large values of position. The subsequent modulo operation may then produce biased results due to this loss of precision.

Impact

This bug can lead to a biased distribution of entropy values. As a result, the generated NFT traits and parameters may not be uniformly distributed as intended. This could potentially:

Create unfair advantages for certain NFTs.
Reduce the overall randomness and uniqueness of the generated NFTs.
Introduce patterns in the NFT traits that could be exploited by savvy players.

The impact is particularly significant given that this entropy is used to determine critical game parameters such as NukeFactor, forgePotential, and role assignment (Forger or Merger).

Proof of Concept

Let's assume slotValue is a large number close to 2^256 - 1, and position is 0.

uint256 slotValue = 2**256 - 1;
uint256 position = 0;

uint256 entropy = (slotValue / (10 ** (72 - position))) % 1000000;

In this case, the integer division slotValue / (10 ** 72) will result in a much smaller number due to truncation. The subsequent modulo operation will then be biased towards smaller values, not providing a uniform distribution across the range 0 to 999999.

Tools Used

Manual review + Calculator

Recommended Mitigation Steps

Use a more robust method for extracting entropy that doesn't rely on division of large numbers. One approach could be to use bitwise operations:
```
uint256 entropy = (slotValue >> (4 * (18 - position))) & 0xFFFFFF;
```
This extracts 24 bits (6 hex digits) from the slotValue at the appropriate position.
If maintaining the current base-10 approach is necessary, consider using a library like ABDKMath64x64 for high-precision arithmetic operations.
Implement thorough testing of the entropy distribution to ensure uniformity across the entire range of possible inputs.

Assessed type

Math

c4-judge commented 1 month ago

koolexcrypto changed the severity to QA (Quality Assurance)