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

Due to poor randomness and incorrect winner selection both parties can gain unfair advantages in `RapBattle::goOnStageOrBattle()`

0xlrivo

Summry

Due to how randomness is generated and how the winner is chosen, both defenders and challengers can get an unfair advantage on the other.

Impact

Severely compomosises the normal battle functionality of the protocol.

Vulnerability Details

The root cause is found in the fact that the "random" number computed inside RapBattle::_battle() is moduled with the sum of the two rapper's skill levels.

uint256 random = uint256(keccak256(abi.encodePacked(block.timestamp, block.prevrandao, msg.sender))) % totalBattleSkill;

Firstly, generating a random number on-chain is dangerous, because any challenger can predict the outcome before the transaction is mined and simply revert if they lose.

The, this "random" number is compared with the defender' skill level and, if <=, the defender wins. This will cause the defender to always have more possibilities to win compared with a challenger in the same situation.

if (random <= defenderRapperSkill) {
// defender wins
}
else {
// challenger wins
}

Tools Used

  • Manual Review

We first need to undestand that the modulo operation, will always return a number between 0 and n-1.

Ex: x % 10 will always return between 0 and 9

For the following scenario we'll consider the worst-case, where one rapper has the default skill level of 50 and the other has the maximum possible of 75:

Equal skill levels

Both defender and challenger have an equal skill level of 50.

skillLevelDefender = 50;
skillLevelAttacker = 50;
totalBattleSkill = 100; // number can range between 0 and 99
​
Defender wins:
0, 1, 2, ..., 50 (51 possibilities)
​
Challenger wins:
51, 52, ..., 99 (49 possibilities)

In this case the defender always have 1 more possibility to win due to the <= condition.

defender's level > challenger's level

Here, defender has an higher skill level than the challenger.

skillLevelDefender = 75;
skillLevelAttacker = 50;
totalBattleSkill = 125; // number can range between 0 and 124
​
Defender wins:
0, 1, 2, ... 75 (76 possibilities)
​
Challenger wins:
76, 77, ..., 124 (49 possibilities)

Here the defender gains 1 more possibility to win!

Defender's level < challenger's level

Here, the challenger has a greated skill level than the defender.

skillLevelDefender = 50;
skillLevelAttacker = 75;
totalBattleSkill = 125; // number can range between 0 and 124
​
Defender wins:
0, 1, 2, ..., 50 (51 possibilities)
​
Challenger wins:
51, 52, ..., 124 (74 possibilities)

Here the challenger has 1 less possibility to win!


In summary, whatever the case, the defender always have more possibilities to win than normally.
But, due to predictable randomness, the challenger can simply revert if he doesn't win.

Reccomended Mitigations

First thing first, the random number must be generated using Chainlink VRF so that it can't be predicted before the transaction is mined.

But, even with VRF in place, the winning check should be changed. We provide two possible soulutions:

  1. After generating the random number add +1 to it

  2. Change the <= with <, without neding the +1 in this case

More informations about Chainlink VRF here

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Weak Randomness

Support

FAQs

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