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

2 stars 1 forks source link

Potential Uninitialized `entropySlots` Reading in `getNextEntropy`, Causing 0 Entropy Mint #1086

Open howlbot-integration[bot] opened 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

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

Vulnerability details

Title

Potential Uninitialized entropySlots Reading in getNextEntropy

Impact

The getNextEntropy function can be called at any time without waiting for the write entropy batches process to finish. This could lead to the function returning an uninitialized entropy value of 000000, resulting in users losing funds to mint useless tokens and not being eligible for future airdrops as they get 0 shares. This vulnerability can severely impact the users' trust and the protocol's functionality.

Proof of Concept


101:  function getNextEntropy() public onlyAllowedCaller returns (uint256) { 
102:    require(currentSlotIndex <= maxSlotIndex, 'Max slot index reached.');
103:@>     uint256 entropy = getEntropy(currentSlotIndex, currentNumberIndex); 
104:
    ...
120:  }

POC

Apply following POC via git apply POC.patch and run yarn test. The test confirms getNextEntropy did return entropy 0 instead of revert.

diff --git a/test/EntropyGenerator.test.ts b/test/EntropyGenerator.test.ts
index 69551f5..f7e8698 100644
--- a/test/EntropyGenerator.test.ts
+++ b/test/EntropyGenerator.test.ts
@@ -3,7 +3,7 @@ import { ethers } from 'hardhat';
 import { EntropyGenerator } from '../typechain-types';
 import { HardhatEthersSigner } from '@nomicfoundation/hardhat-ethers/signers';

-describe('EntropyGenerator', function () {
+describe.only('EntropyGenerator', function () {
   let entropyGenerator: EntropyGenerator;
   let owner: HardhatEthersSigner;
   let allowedCaller: HardhatEthersSigner;
@@ -32,6 +32,15 @@ describe('EntropyGenerator', function () {
     expect(updatedCaller).to.equal(allowedCaller);
   });

+  it('getNextEntropy returns 0 entropy if entropySlots not properly init', async function () {
+    // call getNextEntropy when write entropy batches not finished
+    await expect(
+      entropyGenerator.connect(allowedCaller).getNextEntropy()
+    ).to.emit(entropyGenerator, 'EntropyRetrieved').withArgs(0);
+
+    // @audit: should revert getNextEntropy if entropySlots not properly init
+  });
+
   it('should write entropy batches 1', async function () {
     // Write entropy batch 1
     await entropyGenerator.writeEntropyBatch1();

Tools Used

Hardhat

Recommended Mitigation Steps

Only allow getNextEntropy call if currentSlotIndex < lastInitializedIndex to ensure that the entropy slots are properly initialized:

  function getNextEntropy() public onlyAllowedCaller returns (uint256) {
...
+    require(currentSlotIndex < lastInitializedIndex, 'Slot not initialized');
    uint256 entropy = getEntropy(currentSlotIndex, currentNumberIndex);

Assessed type

Other

c4-judge commented 2 months ago

koolexcrypto marked the issue as not a duplicate

c4-judge commented 2 months ago

koolexcrypto marked the issue as primary issue

c4-judge commented 2 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 2 months ago

koolexcrypto marked the issue as selected for report

Brivan-26 commented 2 months ago

Hi @koolexcrypto Sorry for dropping a comment now, but this is a completely invalid issue. The entropySlots will be initialized just after the deployment. This is confirmed by the deploy script

koolexcrypto commented 2 months ago

Hi @koolexcrypto Sorry for dropping a comment now, but this is a completely invalid issue. The entropySlots will be initialized just after the deployment. This is confirmed by the deploy script

Please check this for more context:

https://github.com/code-423n4/2024-07-traitforge-validation/issues/945

Brivan-26 commented 2 months ago

Thanks @koolexcrypto . The validity of this issue is based on the timeframe after deployment where the entropies are not initialized, as stated by the sponsor:

this is a valid issue as there could be a timeframe where there is not initialised entropy. better yet, we should put a batch in constructor to be sure of mitigation.

However, this information was not accessible during the contest and the deployment script I pointed out above contradicts this fact and demonstrates that entropies will be initiated just after the deployment

Thanks for considering this one more time.

I will not drop further comments