`Golden God` Tokens can be minted twice per generation

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

The current implementation of the EntropyGenerator contract allows for the possibility of having two "Golden God" tokens (tokens with an entropy value of 999999) per generation. This duplicity undermines the intended uniqueness and rarity of the Golden God token, potentially disrupting the game's economy and fairness. The predictability of one Golden God token, combined with the chance of a second one, could lead to exploitation and loss of player trust.

Proof of Concept

The vulnerability stems from two separate mechanisms that can produce a Golden God token:

1. Predetermined Golden God:

   function getEntropy(uint256 slotIndex, uint256 numberIndex) private view returns (uint256) {
   if (slotIndex == slotIndexSelectionPoint && numberIndex == numberIndexSelectionPoint) {
   return 999999;
   }

2.Entropy Calculation from the valued given by writeBatch:

uint256 slotValue = entropySlots[slotIndex];
uint256 entropy = (slotValue / (10 ** (72 - position))) % 1000000;
uint256 paddedEntropy = entropy * (10 ** (6 - numberOfDigits(entropy)));

The predetermined Golden God is set during initialization, and the other Golden God token is set in the writeEntropyBatch3, since the math in the getEntropy doesn't affect 6 figs digits, this means if writeEntropyBatch3 does write a slot with golden number 999999, it's only a matter of time before someone gets since the nfts are attributed the slot sequentially.(token 1 will get slotIndex and numberIndex [0,0], token 2 will get [0,1], token 3 will get [0,2], and so on)

Tools Used

Manual Review

Recommended Mitigation Steps:

add a boolean to check if a golden god token has already been issued

Assessed type

Context

[TForge1]

although it is technically an issue. it is definitely not high risk. Low at most as it doesn't affect the game that much, just doesn't make the golden god as cool.

[koolexcrypto]

Based on the given criteria, This would be categorized as Medium.

Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements

https://docs.code4rena.com/awarding/judging-criteria/severity-categorization

Reasoning:

• Assets are not at direct risk, which rules out a High severity categorization.
• The function of the protocol is impacted, as the uniqueness and rarity of the "Golden God" token are compromised.
• The issue could impact the game fairness, which are core aspects of the protocol's intended function.

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report

[Brivan-26]

This is an intended behavior and a design choice. It is explicitly mentioned in the docs that Golden entropy can be selected at random slots and indexes:

The getEntropy function calculates an entropy value based on a given slot and number index. It first verifies that the provided slot index is within bounds. If the slot and number indices match the predefined selection points, a special value of 999999 is returned. Otherwise, it computes the position for slicing the entropy value from the selected slot in the entropySlots array

[Welith]

Hello @koolexcrypto it seems that one of my low issues got stuck in the <details> markdown of my NC issues in my QA report, which was judged to be unsatisfactory -> https://github.com/code-423n4/2024-07-traitforge-validation/blob/main/data/0xb0k0-Q.md#l-3-the-golden-god-nft-can-appear-twice-in-one-generation. I have also reported that the golden god can appear twice due to the entropy generation calculations, however, the issue is in the Instances details part of the last NC issue Internal functions called only once can be inlined. In defense of the validity of this issue, if we get two golden gods in one generation, it messes up the game's validity, as I've discussed with the sponsor, and he confirmed that each generation should have only one golden god. It seems that I've not read all of C4's rules about what a medium issue is (the game fairness part), which is why I submitted is as low.

[koolexcrypto]

This is an intended behavior and a design choice. It is explicitly mentioned in the docs that Golden entropy can be selected at random slots and indexes:

The getEntropy function calculates an entropy value based on a given slot and number index. It first verifies that the provided slot index is within bounds. If the slot and number indices match the predefined selection points, a special value of 999999 is returned. Otherwise, it computes the position for slicing the entropy value from the selected slot in the entropySlots array

I don't see where it mentions two Gods are allowed (even implicitly).

[koolexcrypto]

Hello @koolexcrypto it seems that one of my low issues got stuck in the <details> markdown of my NC issues in my QA report, which was judged to be unsatisfactory -> https://github.com/code-423n4/2024-07-traitforge-validation/blob/main/data/0xb0k0-Q.md#l-3-the-golden-god-nft-can-appear-twice-in-one-generation. I have also reported that the golden god can appear twice due to the entropy generation calculations, however, the issue is in the Instances details part of the last NC issue Internal functions called only once can be inlined. In defense of the validity of this issue, if we get two golden gods in one generation, it messes up the game's validity, as I've discussed with the sponsor, and he confirmed that each generation should have only one golden god. It seems that I've not read all of C4's rules about what a medium issue is (the game fairness part), which is why I submitted is as low.

Please provide the link of your QA report

[Welith]

@koolexcrypto thanks for the response. Here is the link to my QA report issue. The issue is from the <details> tag of the NC-4 issue I've reported. I've somehow mixed up the closing tags, so my low issues have been included in NC-4.

[Brivan-26]

This is an intended behavior and a design choice. It is explicitly mentioned in the docs that Golden entropy can be selected at random slots and indexes:

The getEntropy function calculates an entropy value based on a given slot and number index. It first verifies that the provided slot index is within bounds. If the slot and number indices match the predefined selection points, a special value of 999999 is returned. Otherwise, it computes the position for slicing the entropy value from the selected slot in the entropySlots array

I don't see where it mentions two Gods are allowed (even implicitly).

Golden entropy is generated on the third batch, so it exists on the entropySlots for every generation. The documentation I linked explicitly says that when the function getEntropy is called, it can return 999999 directly without consulting the entropySlots state which makes the fact that golden entropies can be found twice per generation

[Welith]

This is an intended behavior and a design choice. It is explicitly mentioned in the docs that Golden entropy can be selected at random slots and indexes:

The getEntropy function calculates an entropy value based on a given slot and number index. It first verifies that the provided slot index is within bounds. If the slot and number indices match the predefined selection points, a special value of 999999 is returned. Otherwise, it computes the position for slicing the entropy value from the selected slot in the entropySlots array

I don't see where it mentions two Gods are allowed (even implicitly).

Golden entropy is generated on the third batch, so it exists on the entropySlots for every generation. The documentation I linked explicitly says that when the function getEntropy is called, it can return 999999 directly without consulting the entropySlots state which makes the fact that golden entropies can be found twice per generation

That is where the issue comes from (also confirmed by the sponsor), that if 999999 has been provided once, it should not be possible anymore. I am attaching my correspondence with the protocol team regarding this one: