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);
}