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

The `RapBattle::Battle` event is triggered with wrong data (wrong winner) if the random value is equal to the defenderRapperSkill

Sahil

Summary

The RapBattle::Battle event is triggered with wrong data (wrong winner) if the random value is equal to the defenderRapperSkill. The event is emitted with it's third field indicating the winner to be the challenger, however, the contract picks the defender as the winner.

Vulnerability Details

The RapBattle::_battle function emits the RapBattle::Battle event with it's third field calculated using the less than relational operator (<).

67. random < defenderRapperSkill ? _defender : msg.sender

However, in the next few lines, we see the usage of less than or equal to relational operator (<=) to pick the winner:

70. if (random <= defenderRapperSkill)

Thus, if the random value evaluates to be equal to defenderRapperSkill, the data in the event emitted and the actual winner picked will be different. The winner indicated by the event will be the challenger, but the winner picked by the contract will be the defender.

Impact

Events capture important information, allowing external observers to react to them and obtain relevant data. Dapps use events to display and update data in their UIs. An event emitted with wrong data may confuse or mislead people.

Tools Used

Foundry, VSCodium.

Recommendations

Make the following changes in the RapBattle::_battle function:

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;
oneShot.approve(address(rapBattle), 1);
cred.approve(address(rapBattle), 1);
​
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);
+ 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);
}
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.