Submission Details
Severity:
[L-1] The formula determining the winner in `RapBattle::_battle` is not the same everywhere, causing a low chance to emit an event with the wrong winner
Summary
Emitted winner can not be the same as the real winner.
Vulnerability Details
The winner emitted can be different than the real winner because the formula used to emit the winner is not the same as the formula used to choose the winner.
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(
// @reviewed-high not random
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);
// @audit-high the attaker can vulunteraly not approve before and make the
// transaction fails each time he battle
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 emmited event can have the wrong winner, causing a display of the wrong winner for a Dapp usage.
Tools Used
Foundry
Recommendations
change the function _battle as below :
function _battle(uint256 _tokenId, uint256 _credBet) internal {
...
+ bool defenderWinBattle = random <= defenderRapperSkill ? true : false;
emit Battle(
msg.sender,
_tokenId,
- random < defenderRapperSkill ? _defender : msg.sender
+ defenderWinBattle ? _defender : msg.sender
);
// If random <= defenderRapperSkill -> defenderRapperSkill wins, otherwise they lose
- if (random <= defenderRapperSkill) {
+ if (defenderWinBattle) {
// We give them the money the defender deposited, and the challenger's bet
credToken.transfer(_defender, defenderBet);
// @audit-high the attaker can vulunteraly not approve before and make the
// transaction fails each time he battle
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 over 1 year ago
Submission Judgement Published Assigned finding tags:
Contradictory battle result event
Rewards breakdown
nSLOC
201This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.