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

Challengers Can Battle Without Putting their `CredToken`s at Risk

EloiManuel

Summary

As a challenger, it is possible to participate in battles without putting CredToken tokens at stake by simply not approving the RapBattle contract to spend one's tokens. This way, if the battle is lost, the entire transaction will revert.

Vulnerability Details

The code snippet below contains the logic for determining the winner:

// 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);
}

Notice that, if the defender wins and the challenger has not approved the RapBattle contract to spend its CredToken, the transaction will fail (effectively canceling the battle).

Impact

This is considered to be high risk because it defeats the whole purpose of RapBattle and CredToken (as indicated in the documentation, it is "The primary currency at risk in a rap battle").

Proof of Concept

In order to observe the behavior explained above, add the following test to test/OneShotTest.t.sol:

function testChallengerNoRisk() public mintRapper {
// In order to use 1 cred, lets stake my rapper for 1 day
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(1 days + 1);
streets.unstake(0);
cred.approve(address(rapBattle), 1);
oneShot.approve(address(rapBattle), 0);
rapBattle.goOnStageOrBattle(0, 1);
vm.stopPrank();
address user2 = user = makeAddr("Bob");
vm.startPrank(user2);
rapBattle.goOnStageOrBattle(99, 1);
vm.stopPrank();
}

And run it with forge test -vvvv --mt testChallengerNoRisk. Observe that when the defender (Alice) is the winner, the test will fail because the challenger (Bob) did not approve RapBattle to spend enough of his CredToken tokens.

Tools Used

Manual analysis and Foundry.

Recommendations

Challengers should also be required to send their CredToken tokens when participating in a battle.

Updates

Lead Judging Commences

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

missing check for sufficient `_credBet_` approval

Support

FAQs

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