First Flight #10: One Shot
First Flight #10
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Inconsistency Between Event Logs and Actual Battle Results

EloiManuel

Summary

In the contract RapBattle, the _battle() function contains the logic for deciding who wins a rap battle. When this happens, an event log is emitted containing the winner and the battle prize is distributed. Even though they should always be consistent, there is a case that would make the event indicate that the winner is not the same as the address that receives the prize.

Vulnerability Details

Notice in the code snippet below that in the case when random is equal to defenderRapperSkill, the event would indicate that the winner is msg.sender (aka challenger) while the one receiving the prize would be _defender (the defender).

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

Impact

The RapBattle::Battle is an important event. The incorrect event logs may cause off-chain services to malfunction.

Tools Used

Manual analysis.

Recommendations

It is recommended to fix the inconsistency between the logs and actual logic as shown below:

- 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) {

or

emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
​
- // If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
- if (random <= defenderRapperSkill) {
+ // If random < defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
+ if (random < defenderRapperSkill) {
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Contradictory battle result event

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.