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

Lack of handling for equal skill in the `RapBattle::_battle` function thus leading to an undesired outcome for contesters.

oxenzo

Description: The RapBattle::_battle function does not include logic to handle the scenario where the randomly generated random value is equal to defenderRapperSkill. This means that when random equals defenderRapperSkill, indicating a tie or draw in the battle, the contract's behavior is undefined.

​
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;
​
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
​
// As seen there is no explicit check if the random value is equal to the defenderRapperSkill, instead there is a check for less or equal to but even if there is a draw the defender still gets rewarded both bets which is quite unfair.
@> if (random <= defenderRapperSkill) {
credToken.transfer(_defender, defenderBet);
credToken.transferFrom(msg.sender, _defender, _credBet);
} else {
credToken.transfer(msg.sender, _credBet);
}
totalPrize = 0;
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
​

Detail: As seen in the highlighted part of the function, there is no explicit check if the random value is equal to the defenderRapperSkill, instead there is a check for less or equal to but even if there is a draw the defender still gets rewarded both bets which is quite unfair.

Impact: The lack of handling for equal skill could lead to confusion or undesired outcomes for users such as the bearing the reward of both bets even though there was a draw.

Proof of Concept:

  1. When the _battle function is called with the random equal to defenderRapperSkill, If both let's say are equal to 50.

  2. The function's logic does not handle the scenario, leading to an unfair outcome whereby the defender address gets to win the total of both bets in a draw situation.

Recommended Mitigation: Introduce logic to handle the case where random is equal to defenderRapperSkill

​
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;
​
defender = address(0);
emit Battle(msg.sender, _tokenId, random < defenderRapperSkill ? _defender : msg.sender);
​
- if (random <= defenderRapperSkill) {
- credToken.transfer(_defender, defenderBet);
- credToken.transferFrom(msg.sender, _defender, _credBet);
- } else {
- credToken.transfer(msg.sender, _credBet);
- }
​
// Handle Equal Skill
+ if (random == defenderRapperSkill) {
// Refund the bets to both players
+ credToken.transfer(msg.sender, _credBet);
+ credToken.transfer(_defender, defenderBet);
+ } else if (random < defenderRapperSkill) {
// Defender wins
+ credToken.transfer(_defender, defenderBet);
+ credToken.transferFrom(msg.sender, _defender, _credBet);
+ } else {
// Challenger wins
+ credToken.transfer(msg.sender, _credBet);
+ }
totalPrize = 0;
oneShotNft.transferFrom(address(this), _defender, defenderTokenId);
}
​

Explanation:

  1. Added a check for random == defenderRapperSkill to handle the tie scenario.

  2. If random equals defenderRapperSkill, both the defender and the challenger will receive a refund of their bets. This ensures that the contract handles all possible outcomes of the battle, including ties, in a fair and defined manner.

Final Note: Implementing this mitigation ensures that the contract behavior is consistent and fair, preventing unexpected outcomes and providing a clear resolution for tie scenarios in the battle.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Defender's advantage

Support

FAQs

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