Summary
The RapBattle::_battle() function can emit incorrect event parameters and make it appear that the challenger won the battle when in actuality the defender has won.
Vulnerability Details
The RapBattle::_battle() function emits the Battle event with 3 parameters, the challenger (msg.sender), the _tokenId, and the winner of the battle:
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);
}
The winner of the battle is determined by the condition that if random is less than the defenderRapperSkill then the event emits with the _defender as the winner. Where as if random is greater than defenderRapperSkill, then the event emits with the msg.sender (the challenger) as the winner.
This statement contradicts the following condition after the event is emitted that determines who wins the funds. As that statement determines the winner on the condition that if random is less than or equal to the defenderRapperSkill then the _defender wins the tokens.
Due to the mismatch of conditions, there is a scenario where if the random number generated is equal to the defenderRapperSkill, the event emitted will emit with msg.sender (the challenger) as the winner but the funds will get transferred to the _defender.
Impact
POC
Below you can see a POC that shows how this can happen:
function testIncorrectBattleEvent() public twoSkilledRappers {
uint256 DEFENDER_AMOUNT_TO_BET = cred.balanceOf(user);
uint256 CHALLENGER_AMOUNT_TO_BET = cred.balanceOf(challenger);
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), DEFENDER_AMOUNT_TO_BET);
rapBattle.goOnStageOrBattle(0, DEFENDER_AMOUNT_TO_BET);
vm.stopPrank();
vm.startPrank(challenger);
vm.warp(54 seconds);
oneShot.approve(address(rapBattle), 1);
cred.approve(address(rapBattle), CHALLENGER_AMOUNT_TO_BET);
vm.recordLogs();
rapBattle.goOnStageOrBattle(1, CHALLENGER_AMOUNT_TO_BET);
vm.stopPrank();
Vm.Log[] memory entries = vm.getRecordedLogs();
address winner = address(uint160(uint256(entries[0].topics[2])));
assert(winner == challenger);
assert(cred.balanceOf(challenger) == 0);
assert(
cred.balanceOf(user) ==
DEFENDER_AMOUNT_TO_BET + CHALLENGER_AMOUNT_TO_BET
);
}
Tools Used
VS Code, Foundry
Recommendations
Update the condition of the determined winner when emitting the event:
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);
+ 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);
}
