Summary
In the RapBattle contract the Battle event emits the challanger as the winner when the calculated random number equals to the defenderRapperSkill however in this case the real winner is the defender.
Vulnerability Details
RapBattle::_battle Line 67-77:
@> 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);
}
The conditional statements in the two highlighted lines are different.
Impact
dApps that relies on this event can gets incorrect winner.
Tools Used
Manual review
Recommendations
Use the correct condition in the event.
- 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);
}