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

Inconsistency between the condition used to determine the winner of a battle in the `RapBattle::_battle` function and the condition specified in the `Battle` event

bube

Summary

The RapBattle smart contract contains an inconsistency between the condition used to determine the winner of a battle in the _battle function and the condition specified in the Battle event emission.

Vulnerability Details

The RapBattle::_battle function performs the battle between the rappers tokens and transfer the reward to the winner.
In the comment is written: If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose, but in the event Battle the winner is defined in another way: random < defenderRapperSkill ? _defender : msg.sender.

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

The event Battle in the RapBattle::_battle function can emit an incorrect winner. The inconsistency between the winning condition in the if statement and the Battle event could confuse off-chain services and users interpreting the battle outcomes.

Tools Used

Manual Review

Recommendations

Change the condition in the Battle event to be the same as the condition in the if statement.

Updates

Lead Judging Commences

inallhonesty Lead Judge almost 2 years 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.

Give us feedback!