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

```RapBattle:Battle``` event is emitted wrong in case ```random``` value is equal to the ```defenderRapperSkill```

kiteweb3

Summary

The RapBattle::_battle has a discrepancy in how the battle outcome is determined and reported. Specifically, the event Battle is emitted with the challenger (msg.sender) as the winner when the random value is equal to the defenderRapperSkill, despite the actual logic determining the defender as the winner.

Vulnerability Details

The vulnerability lies in the inconsistency between the event emission and the actual logic used to determine the battle winner. The Battle event is emitted with msg.sender as the winner when the random value is equal to defenderRapperSkill, which contradicts the subsequent if statement that determines the defender as the winner based on the same condition.

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

Impact

This inconsistency can impact the transparency and trustworthiness of the contract. Users rely on the event data to understand the outcome of battles, and the discrepancy could lead to confusion.

Tools Used

Manual review.

Recommendations

Adjust the event to accurately reflect the outcome determined by the if statement.

- emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
+ emit Battle(msg.sender, _tokenId, random <= defenderRapperSkill ? _defender : msg.sender);
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.