Users with minting passes can mint fighters that are both Dendroids and Icons
c4-bot-6 opened this issue · 5 comments
Lines of code
Vulnerability details
Impact
There are two types of fighters: Champions and Dendroids.
Icons are a rare subtype of champions. On the other hand Dendroids are a are a more exclusive class of NFTs, which are rarer than champions.
But it is possible to mint a Dendroid that is also an Icon. This breaks the previously mentioned invariant that an icon is a subtype of champion.
NFTs are valued by their rarity and special traits. In this case, it is possible to mint one with attributes that should not co-exist, which can lead to an unfair pricing in markets, and potential glitches on the game given the unexpected combination. A Medium severity was assessed given the broken invariant and the given reasons.
Vulnerability Details
Users can mint new fighters via redeemMintPass()
.
They can provide both the fighterType
, and the iconsType
.
Those values aren't validated in the function, nor on any other internal call (as proved on the POC). That means it is possible to create a Champion (fighterType == 0
) that is also an Icon (iconsType > 0
).
Reference:
Proof of Concept
- Add the test to
test/FighterFarm.t.sol
- Run
forge test --mt "testRedeemMintPassIcons"
function testRedeemMintPassDendroidIcon() public {
// Claim mint pass and approve it to _fighterFarmContract
uint8[2] memory numToMint = [1, 0];
bytes memory signature = abi.encodePacked(hex"20d5c3e5c6b1457ee95bb5ba0cbf35d70789bad27d94902c67ec738d18f665d84e316edf9b23c154054c7824bba508230449ee98970d7c8b25cc07f3918369481c");
string[] memory _tokenURIs = new string[](1);
_tokenURIs[0] = "ipfs://bafybeiaatcgqvzvz3wrjiqmz2ivcu2c5sqxgipv5w2hzy4pdlw7hfox42m";
_mintPassContract.claimMintPass(numToMint, signature, _tokenURIs);
_mintPassContract.approve(address(_fighterFarmContract), 1);
// Build parameters to pass to redeemMintPass()
uint256[] memory _mintpassIdsToBurn = new uint256[](1);
string[] memory _mintPassDNAs = new string[](1);
uint8[] memory _fighterTypes = new uint8[](1);
uint8[] memory _iconsTypes = new uint8[](1);
string[] memory _neuralNetHashes = new string[](1);
string[] memory _modelTypes = new string[](1);
_mintpassIdsToBurn[0] = 1;
_mintPassDNAs[0] = "dna";
_fighterTypes[0] = 1; // @audit Dendroid
_neuralNetHashes[0] = "neuralnethash";
_modelTypes[0] = "original";
_iconsTypes[0] = 1; // @audit Icon
// The fighter was successfully minted as a Dendroid and Icon
_fighterFarmContract.redeemMintPass(
_mintpassIdsToBurn, _fighterTypes, _iconsTypes, _mintPassDNAs, _neuralNetHashes, _modelTypes
);
// Verify that the fighter is both a Dendroid and an Icon
(,,,,,,, uint8 iconsType, bool dendroidBool) = _fighterFarmContract.fighters(0);
assertEq(dendroidBool, true);
assertEq(iconsType, 1);
}
Recommended Mitigation Steps
Validate that the minted fighter can't be a Dendroid and an Icon at the same time:
+ if (fighterType == 1 && iconsType > 0) {
+ revert();
+ }
fighterType == 1
=> DendroidiconsType > 0
=> Icon
Assessed type
Invalid Validation
raymondfam marked the issue as sufficient quality report
raymondfam marked the issue as duplicate of #33
raymondfam marked the issue as duplicate of #1626
HickupHH3 changed the severity to 3 (High Risk)
HickupHH3 marked the issue as satisfactory