Closed c4-bot-9 closed 2 months ago
I respectfully request a reconsideration of this finding.
As evidenced in the deployment script, the TraitForgeNft contract is fully set up and ready for the minting process prior to the start of the Writing EntropyBatch operations. This sequence highlights a significant risk that must be addressed.
task('deploy-all', 'Deploy all the contracts').setAction(async (_, hre) => {
await nft.setEntropyGenerator(await entropyGenerator.getAddress());
await nft.setEntityForgingContract(await entityForging.getAddress());
await nft.setNukeFundContract(await nukeFund.getAddress());
await nft.setAirdropContract(await airdrop.getAddress());
await entityTrading.setNukeFundAddress(await nukeFund.getAddress());
await entityForging.setNukeFundAddress(await nukeFund.getAddress());
await airdrop.setTraitToken(await token.getAddress());
await airdrop.transferOwnership(await nft.getAddress());
console.log('Generating Merkle Tree...');
const { rootHash } = generateMerkleTree(WHITELIST);
await nft.setRootHash(rootHash);
console.log('Generated & Set the root hash successfully.');
...
console.log('Writing EntropyBatch...');
const tx1 = await entropyGenerator.writeEntropyBatch1();
tx1.wait();
const tx2 = await entropyGenerator.writeEntropyBatch2();
tx2.wait();
const tx3 = await entropyGenerator.writeEntropyBatch3();
tx3.wait();
console.log('Writing EntropyBatch done.');
});
The writeEntropyBatch1,2,3
calls, which populate the entropy, are protected by a safeguard mechanism against an entropy value of 999999
(see EntropyGenerator.sol: Line 56 and EntropyGenerator.sol: Line 76). Should any of these calls revert due to this safeguard, the batch writing process will need to be retried.
However, because the minting setup is already completed and live, there exists a critical window where mint transactions could be processed before the entropy batches are fully written. During this window, any mint transactions would result in 0 entropy (reading of uninitialized entropySlots), leading to unintended outcomes, as detailed in my original finding.
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.
Thank you everyone for the feedback.
will be moved to the main repo as a valid issue
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 ingetNextEntropy
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 of000000
, 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
POC
Apply following POC via
git apply POC.patch
and runyarn test
. The test confirmsgetNextEntropy
did return entropy 0 instead of revert.Tools Used
Hardhat
Recommended Mitigation Steps
Only allow
getNextEntropy
call ifcurrentSlotIndex < lastInitializedIndex
to ensure that the entropy slots are properly initialized:Assessed type
Other