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

2 stars 1 forks source link

The `forgePotential` calculation in the `EntropyGenerator.deriveTokenParameters` function is erroneous #329

Open howlbot-integration[bot] opened 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

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

Vulnerability details

Impact

In the EntropyGenerator.deriveTokenParameters function the forgePotential is calculated as follows:

    forgePotential = getFirstDigit(entropy);

Here the first digit of the entropy value is considered as the forgePotential value.

But the above calculation is erroneous since in the EntityForging.listForForging function the forgePotential was taken as the 5th digit of the entropy as shown below:

    uint8 forgePotential = uint8((entropy / 10) % 10);

The above calculation in the EntityForging.listForForging function can be considered to be correct since it is used for forging of new NFT entities.

As a result the use of the EntropyGenerator.deriveTokenParameters function to retrieve the token parameters will result in erroneous values for forgePotential which would provide wrong forging potential for the NFT entities thus breaking the subsequent logic which is dependent on the forgePotential value.

Proof of Concept

    forgePotential = getFirstDigit(entropy);

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

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to correct the forgePotential calculation in the EntropyGenerator.deriveTokenParameters function as shown below:

    forgePotential = (entropy / 10) % 10;

Assessed type

Other

c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Out of scope

c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

udsene commented 2 months ago

@koolexcrypto,

The above issue is a clear error in the coding and this contract is in scope. This issue explains the forgePotential calculated in the EntropyGenerator.deriveTokenParameters function is erroneous. The above is a public view function and any user could call it and end up receiving the wrong forgePotential thus resulting in wrong conclusions about the token parameters. Hence I would like to request you to reconsider the severity of this issue being Medium or at least a QA.

Further if this issue is considered as a QA then i think it should be linked with my QA Report https://github.com/code-423n4/2024-07-traitforge-findings/issues/1011 which is currently evaluated as grade-a.

Thank You

c4-judge commented 2 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

koolexcrypto commented 2 months ago

Thank you for your feedback.

This can be a QA. all dups are adjusted accordingly.